Use GlobalState, use LocalizationDirection, less var's #1155
Conversation
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-pebble-0dc79cb03-1155.westeurope.3.azurestaticapps.net |
src/Core/Components/DesignSystemProvider/FluentDesignTheme.razor.cs
Outdated
Show resolved
Hide resolved
| private readonly JsonSerializerOptions JSON_OPTIONS = new JsonSerializerOptions | ||
| { | ||
| PropertyNameCaseInsensitive = true, | ||
| }; | ||
|
|
||
| [Inject] | ||
| private GlobalState GlobalState { get; set; } = default!; |
There was a problem hiding this comment.
Do you think this is a good time to rename GlobalState to something more appropriate?
For me, a "State" is not a color or theme management system. A name like GlobalTheme or GlobalDesign would be more appropriate. No?
There was a problem hiding this comment.
I think renaming is a good thing to do but it would be a breaking change so not appropriate now.
I like GlobalDesign! Let's reserve that one for when we do want to do the rename. I'll update the source documentation that this is something we are going to do in the next major version.
There was a problem hiding this comment.
You could create a GlobalDesign class and set Obsolete all usages of GlobalState (but keeping the property). A lot of references?
src/Core/Components/DesignSystemProvider/FluentDesignTheme.razor.cs
Outdated
Show resolved
Hide resolved
|
|
||
| var themeJSON = await Module.InvokeAsync<string>("addThemeChangeEvent", _dotNetHelper, Id); | ||
| var theme = themeJSON == null ? null : JsonSerializer.Deserialize<DataLocalStorage>(themeJSON, JSON_OPTIONS); | ||
| string? themeJSON = await Module.InvokeAsync<string>("addThemeChangeEvent", _dotNetHelper, Id); |
There was a problem hiding this comment.
A reason why you changed var to string? and DataLocalStorage? ?
There was a problem hiding this comment.
I don't like var :).
In VS it displays like this (for me based on my settings, only changed it for the first line):
If the type is already known an shown, then why not us it? VS actually does the substitution for you if you double check on the type.
There was a problem hiding this comment.
It shouldn't depend on preferences (like or dislike), but on naming convention 😀
Here's the one we use in many MS projects: https://dvoituron.com/naming-conventions/rules/
The code is clearer / aligned with var and it's more " scalable " when the objects (right) evolve.
But it's only a naming convention.
var themeJSON = ...
var theme = ...| @@ -171,7 +178,7 @@ private async Task ApplyLocalStorageValues(DataLocalStorage? theme) | |||
| // Mode (Dark / Light / System) | |||
| if (!string.IsNullOrEmpty(theme?.Mode)) | |||
| { | |||
| if (!Enum.TryParse<DesignThemeModes>(theme.Mode, true, out var mode)) | |||
| if (!Enum.TryParse(theme.Mode, true, out DesignThemeModes mode)) | |||
There was a problem hiding this comment.
Same... why did you remove var? :-)
Fix compiler warnings
Fix parsing of Luminance value
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-pebble-0dc79cb03-1155.westeurope.3.azurestaticapps.net |
As discussed