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

Make Realtime Graphs Respect Bootstrap Dark Mode #6982

Merged
merged 1 commit into from
Mar 12, 2024
Merged

Make Realtime Graphs Respect Bootstrap Dark Mode #6982

merged 1 commit into from
Mar 12, 2024

Conversation

JimMatthew
Copy link
Contributor

@JimMatthew JimMatthew commented Mar 12, 2024

Currently, the realtime graphs, under Status > Realtime Graphs are always white, even in Bootstrap Dark theme.

This change will make the graphs dark in darkmode. I wanted to get some guidance on whether this is the right direction.
I used !important to set the color because I needed to override the JS which is setting the background white directly. I looked at fixing this in the javascript but there doesn't seem to be common variables among the different themes, so this way seemed the simplest.

This is my first PR so please be patient if I have done anything incorrectly.

Example of dark graph.
Currently the graph lines do not show up well, I can work on that if requested.

darkmodeGraph

@systemcrash
Copy link
Contributor

Looks good! Just pay attention to other PR checks and resolve those. Squash your changes into a single commit and force push. Remember your signed-off-by line in the commit message.

@JimMatthew
Copy link
Contributor Author

Looks good! Just pay attention to other PR checks and resolve those. Squash your changes into a single commit and force push. Remember your signed-off-by line in the commit message.

Thanks, did as requested.

@systemcrash
Copy link
Contributor

Almost! Just add subject line to commit, line should start like "themes: ..."

This feature is to have the realtime graphs dark when using Bootstrap Dark Theme

Signed-off-by: james <[email protected]>
@systemcrash systemcrash merged commit 0564077 into openwrt:master Mar 12, 2024
1 check passed
@systemcrash
Copy link
Contributor

Good! Thanks.

@thomasschroeder
Copy link
Contributor

Great update, would be highly beneficial for the "Channel Analysis" page as well.

One more suggestion: Changing the colour of the grid to the one of the table grid lines would make it much easier to read.
grafik

@JimMatthew
Copy link
Contributor Author

@systemcrash if I make improvements do I make a new pull request?

@JimMatthew
Copy link
Contributor Author

JimMatthew commented Mar 14, 2024

Great update, would be highly beneficial for the "Channel Analysis" page as well.

One more suggestion: Changing the colour of the grid to the one of the table grid lines would make it much easier to read. grafik

@thomasschroeder what do you think of this? I left the border black because imo it looks better
darkmodeGraph2

@thomasschroeder
Copy link
Contributor

thomasschroeder commented Mar 14, 2024

@thomasschroeder what do you think of this? I left the border black because imo it looks better darkmodeGraph2

Looks good to me!

But one additional point - it seems like the wireless tab is missing in your original PR:
grafik

Update: For the Wireless tab at real time graphs it would be
[data-darkmode="true"] [data-page="admin-status-realtime-wireless"] #view>div>div>div>div[style]

And for the WiFi channel analysis:
[data-darkmode="true"] [data-page="admin-status-channel_analysis"] #view>div>div>div>div>div[style]
grafik

@JimMatthew
Copy link
Contributor Author

thanks, I don't use wireless so my build doesn't have those tabs

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.

3 participants