Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improve text color in Dark mode #2822

Closed
markoweb2 opened this issue Oct 17, 2024 · 4 comments · Fixed by #2828
Closed

feat: improve text color in Dark mode #2822

markoweb2 opened this issue Oct 17, 2024 · 4 comments · Fixed by #2828
Labels
closed:done Work is finished

Comments

@markoweb2
Copy link

🙋 Feature Request

The Color.Success value does not change when switching to Dark mode. Text is rather unreadable.

🤔 Expected Behavior

Would expect it to change to a brighter color, to make it more readable in Dark mode.

😯 Current Behavior

<FluentLabel>This is good readable text</FluentLabel>
<FluentLabel Color="@Color.Success">This text is unreadable in dark mode.</FluentLabel>

In normal Light mode, everything is good:
Image

Switching to Dark mode, the Color.Success value does not change at all, I can barely read the green text:
Image

💁 Possible Solution

Update the --success variable to CSS color: forestgreen in Dark mode (one should not have to dwelve into DesignTokens and changing the entire color palette for this). This is definently a step up from the current behavior and matches the Color.Error tone and readability.
Image

Although both could be improved further by using color: limegreen and color: orangered for their Dark mode values:
Image

🔦 Context

PS! One cannot use Color="@Color.Custom" with FluentLabel, because the property CustomColor does not exist. Either add it or write in the FluentLabel documentation, to use the Style property with CSS color: somecolor if you want to apply custom color.

@microsoft-github-policy-service microsoft-github-policy-service bot added the triage New issue. Needs to be looked at label Oct 17, 2024
@markoweb2
Copy link
Author

Using the FluentDesignTheme component to switch to Dark mode.

@vnbaaij vnbaaij added community:good-first-issue Good issues for first time contributors and removed triage New issue. Needs to be looked at labels Oct 17, 2024
@vnbaaij
Copy link
Collaborator

vnbaaij commented Oct 17, 2024

PS! One cannot use Color="@Color.Custom" with FluentLabel, because the property CustomColor does not exist. Either add it or write in the FluentLabel documentation, to use the Style property with CSS color: somecolor if you want to apply custom color.

We are looking at adding it to label for the next version. In the meantime an update to the docs should suffice. We do take PR's you know. Care to contribute?

@vnbaaij
Copy link
Collaborator

vnbaaij commented Oct 18, 2024

Added CustomColor implementation. See PR mentioned above.

@vnbaaij vnbaaij closed this as completed Oct 18, 2024
@vnbaaij vnbaaij added closed:done Work is finished and removed community:good-first-issue Good issues for first time contributors labels Oct 18, 2024
@markoweb2
Copy link
Author

In case someone actually wants a solution/workaround to the problem stated.
In my MainLayout.razor I have the FluentDesignTheme element (at the top), including a FluentProfileMenu from where I can change theme and color.
I added an OnLuminanceChanged event:

<FluentDesignTheme @bind-Mode="@this.ThemeMode" @[email protected] StorageName="theme" OnLuminanceChanged="this.OnLuminanceChanged" />

@code
{
    private bool isDark = false;

    public DesignThemeModes ThemeMode { get; set; }
    public OfficeColor? OfficeColor { get; set; }

    private void OnLuminanceChanged(LuminanceChangedEventArgs arg)
    {
        this.isDark = arg.IsDark;
    }
}

And then I added in the same file (right below FluentDesignTheme), to override the values:

@if (this.isDark)
{
    <style>
        body {
            --success: limegreen;
            --error: orangered;
        }
    </style>
}

Followed by the rest of the page code:

<div class="page">
    <div class="sidebar">
        <NavMenu />
    </div>
    
    <main>
        <div class="top-row px-4">
            <FluentProfileMenu ... />
        </div>

        <article class="content px-4">
            @Body
        </article>
    </main>
</div>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed:done Work is finished
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants