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

Fixes an issue with dark modes, where observation "hi/Lo" color values (if specified) in skin.conf are not honored. #9

Merged
merged 2 commits into from
Nov 16, 2022

Conversation

W0CHP
Copy link
Collaborator

@W0CHP W0CHP commented Nov 12, 2022

Noticed that in dark mode, the "hi/lo" colors if defined in skin.conf are not honored/rendered. This minor PR fixes that.

@Pogs2004
Copy link
Collaborator

Pogs2004 commented Nov 13, 2022

I noticed this the other day, updated the CSS and it's working fine, many thanks! The default blue doesn't look good against the dark theme background so I've adjusted the colour in the skin.con

@Pogs2004
Copy link
Collaborator

I would suggest changing the default lo_value_color in skin.conf to #4b94eb which can be read easily in both light and dark modes:

        # Here you can set a hex code for the text color of low / high values
        # on all cards. By default they are grey. (quotes are needed on values)
        # lo_value_color = "#03a9f4"
        # hi_value_color = "#f44336"
        lo_value_color = "#4b94eb"
        hi_value_color = red

Copy link
Collaborator

@Pogs2004 Pogs2004 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default lo_value_color = blue does not look good in dark mode. Suggest changing skin.conf appearance section to:

        # Here you can set a hex code for the text color of low / high values
        # on all cards. By default they are grey. (quotes are needed on values)
        # lo_value_color = "#03a9f4"
        # hi_value_color = "#f44336"
        lo_value_color = "#4b94eb"
        hi_value_color = red

See review comments.

@Pogs2004
Copy link
Collaborator

I'm struggling a bit with Github, it's not a platform I am very familiar with. There are two forks that have had recent activity, @W0CHP and @seehase, are these being merged? I am currently working on code for an interactive sun path diagram that gets the date, time and location from Weewx, when complete how do I get this on Github, do I create a new fork?

@seehausen
Copy link

@Pogs2004 as in the original repo there is not much activity we want to continue working in this fork here.
everything from @W0CHP is already included and merged

@W0CHP
Copy link
Collaborator Author

W0CHP commented Nov 16, 2022

The default lo_value_color = blue does not look good in dark mode. Suggest changing skin.conf appearance section to:

        # Here you can set a hex code for the text color of low / high values
        # on all cards. By default they are grey. (quotes are needed on values)
        # lo_value_color = "#03a9f4"
        # hi_value_color = "#f44336"
        lo_value_color = "#4b94eb"
        hi_value_color = red

See review comments.

Please enter a new PR for this. As this PR fixes a bug (vs. styling tweaks). Thanks!

@W0CHP W0CHP dismissed Pogs2004’s stale review November 16, 2022 12:44

Addresses styling vs. fixing an issue in this PR

@W0CHP W0CHP merged commit df364da into seehase:master Nov 16, 2022
@W0CHP
Copy link
Collaborator Author

W0CHP commented Nov 16, 2022

@Pogs2004 as in the original repo there is not much activity we want to continue working in this fork here. everything from @W0CHP is already included and merged

I agree with @seehausen
I am using @seehausen's fork as "authoritative". My fork is simply for the changes I make and propose, then merge into this one ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants