Skip to content

Commit

Permalink
Fix PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
dvoituron committed Jan 22, 2025
1 parent 84bea9b commit 3adda10
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ protected override async Task OnAfterRenderAsync(bool firstRender)

if (JSModule != null)
{
var margin = Margin.SpacingToStyle();
var padding = Padding.SpacingToStyle();
var margin = Margin.ConvertSpacing();
var padding = Padding.ConvertSpacing();

await JSModule.InvokeVoidAsync(
"setSpacing",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,17 @@ The size changes with an interval of, by default, **4 pixels** (see the CSS vari

## `Margin` and `Padding` component attributes

All **FluentUI Blazor components** contain the `Margin` and `Padding` attributes.
All **FluentUI Blazor components** have the `Margin` and `Padding` parameters.

You can specify a **CSS value** respecting the CSS [padding](https://developer.mozilla.org/docs/Web/CSS/padding)
or CSS [margin](https://developer.mozilla.org/docs/Web/CSS/margin) pattern.
Or you can specify a **class name** like defined above on this page.
Or you can use a **class name** like shown earlier on this page.

> If you use one of these properties to include a complete style (e.g. `style=“margin: 10px;”`)
> then this style will be ignored. The property `Style` can be used to include a complete style.
> The contents of these **Margin** and **Padding** parameters can contain either the **value** of
> these margins or paddings (e.g. `10px`), or the name of a CSS class (e.g. `mt-0`).
> If you use one of these parameters to include a complete style (e.g. `margin: 10px;` or `border: 1px`)
> then this style will be ignored.
> If you wish to do this, you must use the `Style` parameter.
Example:
```html
Expand All @@ -117,7 +120,8 @@ Example:

## Appendix - CSS variables

**Spacing** uses these CSS variables, that can be customized:
**Spacing** uses these CSS variables for the default values.
These values can be customized or overridden in your own CSS files.

```css
--spacingVerticalNone: 0;
Expand Down
8 changes: 4 additions & 4 deletions src/Core/Components/Base/FluentComponentBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ public abstract class FluentComponentBase : ComponentBase, IAsyncDisposable, IFl
/// Gets the class builder, containing the default margin and padding values.
/// </summary>
protected virtual CssBuilder DefaultClassBuilder => new CssBuilder(Class)
.AddClass(Margin.SpacingToStyle().Class)
.AddClass(Padding.SpacingToStyle().Class);
.AddClass(Margin.ConvertSpacing().Class)
.AddClass(Padding.ConvertSpacing().Class);

/// <summary>
/// Gets the style builder, containing the default margin and padding values.
/// </summary>
protected virtual StyleBuilder DefaultStyleBuilder => new StyleBuilder(Style)
.AddStyle("margin", Margin.SpacingToStyle().Style)
.AddStyle("padding", Padding.SpacingToStyle().Style);
.AddStyle("margin", Margin.ConvertSpacing().Style)
.AddStyle("padding", Padding.ConvertSpacing().Style);

/// <inheritdoc />
[Parameter]
Expand Down
8 changes: 4 additions & 4 deletions src/Core/Components/Base/FluentInputBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,15 @@ protected FluentInputBase()
/// Gets the class builder, containing the default margin and padding values.
/// </summary>
protected virtual CssBuilder DefaultClassBuilder => new CssBuilder(Class)
.AddClass(Margin.SpacingToStyle().Class)
.AddClass(Padding.SpacingToStyle().Class);
.AddClass(Margin.ConvertSpacing().Class)
.AddClass(Padding.ConvertSpacing().Class);

/// <summary>
/// Gets the style builder, containing the default margin and padding values.
/// </summary>
protected virtual StyleBuilder DefaultStyleBuilder => new StyleBuilder(Style)
.AddStyle("margin", Margin.SpacingToStyle().Style)
.AddStyle("padding", Padding.SpacingToStyle().Style);
.AddStyle("margin", Margin.ConvertSpacing().Style)
.AddStyle("padding", Padding.ConvertSpacing().Style);

/// <summary>
/// Gets a CSS class string that combines the `Class` attribute and and a string indicating
Expand Down
8 changes: 4 additions & 4 deletions src/Core/Components/Dialog/Services/DialogOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,15 @@ public DialogOptions(Action<DialogOptions> implementationFactory)
/// Gets the class, including the optional <see cref="Margin"/> and <see cref="Padding"/> values.
/// </summary>
internal virtual string? ClassValue => new CssBuilder(Class)
.AddClass(Margin.SpacingToStyle().Class)
.AddClass(Padding.SpacingToStyle().Class)
.AddClass(Margin.ConvertSpacing().Class)
.AddClass(Padding.ConvertSpacing().Class)
.Build();

/// <summary>
/// Gets the style builder, containing the default margin and padding values.
/// </summary>
internal virtual string? StyleValue => new StyleBuilder(Style)
.AddStyle("margin", Margin.SpacingToStyle().Style)
.AddStyle("padding", Padding.SpacingToStyle().Style)
.AddStyle("margin", Margin.ConvertSpacing().Style)
.AddStyle("padding", Padding.ConvertSpacing().Style)
.Build();
}
27 changes: 17 additions & 10 deletions src/Core/Components/Spacing/SpacingGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,23 @@ namespace Microsoft.FluentUI.AspNetCore.Components;
*
* Example, using https://dotnetfiddle.net/
*
* We ran several tests to determine the number of elements (e.g. `.ml-?`) to generate, depending on requirements
* and the size of the CSS file generated. We determined that 8 elements is a good compromise
* Spacing can be up to 32px (positive or negative).
* We ran several tests to determine the number of elements (e.g. .ml-?) to generate, depending on requirements
* and the size of the CSS file generated. We determined that 8 elements, where Spacing can then be set up to 32px
* (either positive or negative), gives a good compromise between options and size of the CSS file.
*
* 1. To generate this CSS content, you can run this command in a C# Console Application:
*
* System.Console.WriteLine(SpacingGenerator.Script); // Default `Count` is 8
*
* 2. To find what file size corresponds to what count, the following code can be run in a C# Console Application.
*
* for (int i = 1; i < 25; i++)
* {
* var script = SpacingGenerator.GenerateScript(i);
* System.Console.WriteLine($" Count = {i:00} => Max spacing size: {i*4}px - File size: {script.Length / 1024} kb.");
* }
*
* Results:
*
* Count = 01 => Max spacing size: 4px - File size: 29 kb.
* Count = 02 => Max spacing size: 8px - File size: 43 kb.
Expand All @@ -41,15 +55,8 @@ namespace Microsoft.FluentUI.AspNetCore.Components;
* Count = 23 => Max spacing size: 92px - File size: 378 kb.
* Count = 24 => Max spacing size: 96px - File size: 397 kb.
*
* for (int i = 1; i < 25; i++)
* {
* var script = SpacingGenerator.GenerateScript(i);
* System.Console.WriteLine($" Count = {i:00} => Max spacing size: {i*4}px - File size: {script.Length / 1024} kb.");
* }
*/

// System.Console.WriteLine(SpacingGenerator.Script);

[System.Diagnostics.CodeAnalysis.SuppressMessage("Globalization", "CA1305:Specify IFormatProvider", Justification = "Non Production Code")]
[System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage(Justification = "Non Production Code")]
internal class SpacingGenerator
Expand Down
34 changes: 17 additions & 17 deletions src/Core/Extensions/SpacingExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,17 @@ public static class SpacingExtensions
/// or a classValue name if the value is not a valid CSS keyword like `auto`, `inherit`, `initial`, ...
/// </summary>
/// <example>
/// - SpacingToStyle("auto") => Style = "auto" Class = ""
/// - SpacingToStyle("10px") => Style = "10px" Class = ""
/// - SpacingToStyle("10px 20px") => Style = "10px 20px" Class = ""
/// - SpacingToStyle("10px 20px 30px") => Style = "10px 20px 30px" Class = ""
/// - SpacingToStyle("mr-0") => Style = "" Class = "mr-0"
/// - SpacingToStyle("my-classValue") => Style = "" Class = "my-classValue"
/// - SpacingToStyle("mr-0 my-classValue") => Style = "" Class = "mr-0 my-classValue"
/// - ConvertSpacing("auto") => Style = "auto" Class = ""
/// - ConvertSpacing("10px") => Style = "10px" Class = ""
/// - ConvertSpacing("10px 20px") => Style = "10px 20px" Class = ""
/// - ConvertSpacing("10px 20px 30px") => Style = "10px 20px 30px" Class = ""
/// - ConvertSpacing("mr-0") => Style = "" Class = "mr-0"
/// - ConvertSpacing("my-classValue") => Style = "" Class = "my-classValue"
/// - ConvertSpacing("mr-0 my-classValue") => Style = "" Class = "mr-0 my-classValue"
/// </example>
/// <param name="value"></param>
/// <returns></returns>
public static (string Style, string Class) SpacingToStyle(this string? value)
public static (string Style, string Class) ConvertSpacing(this string? value)
{
// To optimize multiple calls with the same value
if (string.Equals(_previousSpacingToStyle.Key, value, StringComparison.Ordinal))
Expand All @@ -43,7 +43,7 @@ public static (string Style, string Class) SpacingToStyle(this string? value)

if (value == null)
{
return SaveInCacheAndReturns(value, "", "");
return SaveInCacheAndGet(value, "", "");
}

var values = value.Split(Separators, StringSplitOptions.RemoveEmptyEntries)
Expand All @@ -53,7 +53,7 @@ public static (string Style, string Class) SpacingToStyle(this string? value)
// No value
if (values.Length == 0)
{
return SaveInCacheAndReturns(value, "", "");
return SaveInCacheAndGet(value, "", "");
}

// A keyword cannot be used with other values
Expand All @@ -66,7 +66,7 @@ public static (string Style, string Class) SpacingToStyle(this string? value)
var firstValue = values[0];
if (StyleKeyWords.Contains(firstValue, StringComparer.OrdinalIgnoreCase))
{
return SaveInCacheAndReturns(value, values[0], "");
return SaveInCacheAndGet(value, values[0], "");
}

// In CSS, classValue names cannot start with a digit.
Expand All @@ -77,32 +77,32 @@ public static (string Style, string Class) SpacingToStyle(this string? value)

if (isStyle)
{
return SaveInCacheAndReturns(value, string.Join(' ', AddMissingPixels(values)), "");
return SaveInCacheAndGet(value, string.Join(' ', AddPxWhenNeeded(values)), "");
}

// A `calc` function is a valid style
if (firstValue.StartsWith("calc(", StringComparison.OrdinalIgnoreCase))
{
return SaveInCacheAndReturns(value, value, "");
return SaveInCacheAndGet(value, value, "");
}

// Check if value contains invalid characters for a class name
if (value.IndexOfAny(InvalidCharsInClassName) >= 0)
{
return SaveInCacheAndReturns(value, "", "");
return SaveInCacheAndGet(value, "", "");
}

// Like a class name
return SaveInCacheAndReturns(value, "", value);
return SaveInCacheAndGet(value, "", value);
}

private static (string Style, string Class) SaveInCacheAndReturns(string? value, string styleValue, string classValue)
private static (string Style, string Class) SaveInCacheAndGet(string? value, string styleValue, string classValue)
{
_previousSpacingToStyle = new KeyValuePair<string?, (string, string)>(value, (styleValue, classValue));
return (styleValue, classValue);
}

private static string[] AddMissingPixels(string[] values)
private static string[] AddPxWhenNeeded(string[] values)
{
return values.Select(v => IsNumber(v) ? v + "px" : v).ToArray();
}
Expand Down
8 changes: 4 additions & 4 deletions tests/Core/Extensions/SpacingExtensionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class SpacingExtensionsTests
public void SpacingExtensions_Style(string value, string expectedStyle, string expectedClass)
{
// Arrange & Act
var converted = value.SpacingToStyle();
var converted = value.ConvertSpacing();

// Assert
Assert.Equal(expectedStyle, converted.Style);
Expand All @@ -39,7 +39,7 @@ public void SpacingExtensions_Style(string value, string expectedStyle, string e
public void SpacingExtensions_Class(string value, string expectedStyle, string expectedClass)
{
// Arrange & Act
var converted = value.SpacingToStyle();
var converted = value.ConvertSpacing();

// Assert
Assert.Equal(expectedStyle, converted.Style);
Expand All @@ -57,7 +57,7 @@ public void SpacingExtensions_Class(string value, string expectedStyle, string e
public void SpacingExtensions_Keywords(string value, string expectedStyle, string expectedClass)
{
// Arrange & Act
var converted = value.SpacingToStyle();
var converted = value.ConvertSpacing();

// Assert
Assert.Equal(expectedStyle, converted.Style);
Expand All @@ -70,7 +70,7 @@ public void SpacingExtensions_Keywords(string value, string expectedStyle, strin
public void SpacingExtensions_Invalid(string value)
{
// Arrange & Act
var ex = Assert.Throws<ArgumentException>(() => value.SpacingToStyle());
var ex = Assert.Throws<ArgumentException>(() => value.ConvertSpacing());

// Assert
Assert.Equal("The value cannot contain a CSS keyword and a class name or style value. (Parameter 'value')", ex.Message);
Expand Down

0 comments on commit 3adda10

Please sign in to comment.