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: light mode ui #1580

Merged
merged 23 commits into from
Dec 16, 2023
Merged

feat: light mode ui #1580

merged 23 commits into from
Dec 16, 2023

Conversation

inhabitworker
Copy link
Contributor

Description

Added a light mode with accompanying option under UI Settings, relying on vuetify defaults (removing explicit "dark" attributes in code etc.) and a theme mixin used to close the gap/centralize to extend consistently to chart options and other specific places where needed

Related Tickets & Documents

Fixes #1297

Mobile & Desktop Screenshots/Recordings

image
image
image

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 3

src/store/gui/types.ts Outdated Show resolved Hide resolved
src/App.vue Show resolved Hide resolved
src/App.vue Outdated Show resolved Hide resolved
@meteyou
Copy link
Member

meteyou commented Oct 7, 2023

Thanks for your PR! This is a very popular feature. I just found some small issues:

  • In dark mode, the "toolbar" and the "content" of each panel have different background color and you see the there different areas. For example in your screenshot above from the "Interface Settings", the sidebar will scroll and cut by a white line, where the toolbar starts. I would prefer a border bottom of the toolbar like here in my screenshot:
    (I just added it in the chrome dev tools with border-bottom: 1px solid #ccc;)
    image

  • the snackbar is right now in "dark mode":
    image

  • history panel creates errors because of the changes from the charts

i also add some comments to the code changes. I will be happy to merge this feature then.

@inhabitworker
Copy link
Contributor Author

I will make changes according to your feedback. Apologies as I am new to contributing, I'm wondering if I should be creating some test(s) here, rather than eyeballing? Truthfully I am unsure when it comes to this kind of visual feature. Thank you for reviewing my code.

@meteyou
Copy link
Member

meteyou commented Oct 10, 2023

I will make changes according to your feedback. Apologies as I am new to contributing, I'm wondering if I should be creating some test(s) here, rather than eyeballing? Truthfully I am unsure when it comes to this kind of visual feature. Thank you for reviewing my code.

unfortunately there are no tests in mainsail, because of the api connection. i would like to offer some automatic tests, but unfortunately i can't do it at the moment. unfortunately i can only check it with my eyes...

…ht theme adjustments, further theme mixin coverage of extant default dark styling, charts reactive to theme switch
@github-actions
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 9 4
en.json 0 3
fr.json 90 7
it.json 383 9
pl.json 4 3
tr.json 19 4
zh.json 10 2

@inhabitworker
Copy link
Contributor Author

Sorry for the delay, I will list some of the changes I have made:

  • Returned "dark" attributes, with conditional check. Still not sure that these are necessary (they dictate whether to use dark light stylings within theme, which is what is being adjusted anyway).
  • Initial loading of config.json is made to be synchronous before vue is mounted so there isn't an ugly transition when theme is being resolved. As such, theme is resolved further in this process too (default possibly read from file, potential user setting fetched from moonraker, falling back to dark)
  • Some adjustments to charts on the history page to make sure they properly refresh with theme switches.
  • "snackbars" themed, "dragable" elements in settings also themed, which were previously themed by hard coded css class, now as style.
  • border-bottom on panel title bar (toolbar?)

There's a few little changes here and there, I tried to tie things once again into the theme mixin, so these visual settings can be more easily located in case of other developments

src/main.ts Outdated Show resolved Hide resolved
@meteyou
Copy link
Member

meteyou commented Oct 29, 2023

sry for the late response. can you doublecheck if the dark option is needed? if it is not needed, i would prefer to remove it complete.

i also added a comment to the "default setting load". this cannot work this way, because at this moment, its possible, that there don't exist a moonraker url.

@meteyou
Copy link
Member

meteyou commented Dec 3, 2023

@inhabitworker any update here?

# Conflicts:
#	src/main.ts
#	src/store/gui/index.ts
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 20 0
en.json 0 0

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 20 0
en.json 0 0

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 20 0
en.json 0 0

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 20 0
en.json 0 0

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 20 0
en.json 0 0

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 20 0
en.json 0 0

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 20 0
en.json 0 0

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 20 0
en.json 0 0

@meteyou meteyou merged commit 59c31e7 into mainsail-crew:develop Dec 16, 2023
10 checks passed
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.

Theme Switch
2 participants