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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Live theme changes #1497

Merged
merged 18 commits into from
Nov 24, 2021
Merged

Live theme changes #1497

merged 18 commits into from
Nov 24, 2021

Conversation

RumovZ
Copy link
Collaborator

@RumovZ RumovZ commented Nov 17, 2021

This takes care of theme changes inside Qt so far.
I'm uncertain how to proceed with the webviews. What exactly did you mean by "older pages might work with .nightMode?.", @dae? The two options I can think of are either reloading the entire standard HTML or switching to night mode sensitive CSS and only toggling the class of the doc element.
As for the new pages, just a little reminder about the dsiscussion in #1471, @hgiesel. 馃檪

@RumovZ
Copy link
Collaborator Author

RumovZ commented Nov 17, 2021

Oh, and one other thing: The background of the main window's menu bar's menus remain white when switching to night mode and I don't know why. If anyone had an idea why that might be, I'd appreciate it.

- drop _light_palette, as default_palette serves the same purpose
- save default platform theme, and restore it when switching away
from nightmode
- update macOS light/dark mode on theme switch
- fix unreadable menus on Windows
This is the easy part - CSS styling that uses standard_css or our
css variables should update automatically. The main remaining issue
is JS code that sets colors based on the theme at the time it's run -
eg the graph code, and the editor.
@dae
Copy link
Member

dae commented Nov 18, 2021

Thanks Rumo! I've pushed some fixes and further updates, and it seems to be in a workable state now. We'll want to update the rest of the nightModeKey contexts and check to see if anything has been missed, but let's first wait to see how the discussion on #1471 pans out

The menu issue was because of the QMenuBar styling we were only applying in day mode. Similar issues may appear in other cases where we only set styling for night mode, such as the scrollbar styling - we may need to apply different colours in day mode to work around it.

@dae
Copy link
Member

dae commented Nov 19, 2021

Hmm, how about rootTheme?

/// The current theme that applies to this document/shadow root. When
/// previewing cards in the card layout screen, this may not match the
/// theme Anki is using in its UI.
export const rootTheme = readable(getThemeFromRoot(), (set) => {

I started to rename things to pageTheme, but realised a nice property of documentTheme/rootTheme is that they sort of indicate where they are being set. If you're not so fond of rootTheme, happy for me to go with documentTheme instead?

Naming things is hard 馃槄

@hgiesel
Copy link
Contributor

hgiesel commented Nov 19, 2021

My agenda with pageTheme is that if we used an iframe at some point, the CSS would still be isolated from variables set at document.documentElement, even though the iframe also have a Document at their root. So they would need their own rootTheme or frameTheme, etc. At the same time I read this on MDN:

The Document interface represents any web page loaded in the browser [...]

If we think of Anki as a web app to be, we could also name it appTheme (so many options ^^;)

Once we are going to use themes for any iframes or shadow roots, they will need their own individual stores. For example for shadow roots, they don't have a documentElement, but rather a shadowRoot.host, where we'd set the class or attribute. I think for those, we'd just export a constructor from lib/theme, and put the theme store on the component tree (think editor.editorFields[2].editingArea.rootTheme)

@dae
Copy link
Member

dae commented Nov 19, 2021

Ok, I'll go with pageTheme for now so we can move forward with this - we can always change it again before the stable release if necessary. Assuming @RumovZ is interested, are you happy for him to proceed with updating the remaining nightModeKey references to use pageTheme instead, like in c216353? Once that's done, I presume this will be ok to merge - if there's something else we've overlooked, please let us know.

@dae
Copy link
Member

dae commented Nov 23, 2021

@hgiesel gentle ping in case you missed this

@hgiesel
Copy link
Contributor

hgiesel commented Nov 23, 2021

I didn't catch the question for me, sorry ^^; Sure, he can go ahead. It think .isDark can fully fill the role of the old nightMode.

@RumovZ
Copy link
Collaborator Author

RumovZ commented Nov 23, 2021

I'll take care of it. Thanks, the two of you!

@RumovZ
Copy link
Collaborator Author

RumovZ commented Nov 24, 2021

I had to format manually. Am I doing anything wrong? That's how I understand the docs, but apparently it doesn't work:
grafik

@RumovZ RumovZ marked this pull request as ready for review November 24, 2021 10:14
@dae
Copy link
Member

dae commented Nov 24, 2021

Does the latest main commit fix it?

Thanks for your work on this Rumo, I imagine there'll be quite a few users happy to see this land!

@dae dae merged commit f2173fd into ankitects:main Nov 24, 2021
@RumovZ RumovZ deleted the live-theme-changes branch November 24, 2021 21:21
@RumovZ
Copy link
Collaborator Author

RumovZ commented Nov 24, 2021

Does the latest main commit fix it?

Yes, thank you. 馃檪

Glad I could help!

ZoomRmc added a commit to ZoomRmc/anki that referenced this pull request Sep 14, 2022
ankitects#1497 introduced reading hardcoded Windows Registry key, which assumes key exists. This is not true on Windows 7. Later addition of `try-except` block missed that OpenKey might fail.

This fix allows launching current version of Anki on Windows 7 when installed with Pip with the modified Python 3.9 installation.
ZoomRmc added a commit to ZoomRmc/anki that referenced this pull request Sep 14, 2022
ankitects#1497 introduced reading hardcoded Windows Registry key, which assumes key exists. This is not true on Windows 7. Later addition of `try-except` block missed that OpenKey might fail.

This fix allows launching current version of Anki on Windows 7 when installed with Pip with the modified Python 3.9 installation.
dae pushed a commit that referenced this pull request Sep 15, 2022
* Fix Windows dark mode detection: OpenKey can fail

#1497 introduced reading hardcoded Windows Registry key, which assumes key exists. This is not true on Windows 7. Later addition of `try-except` block missed that OpenKey might fail.

This fix allows launching current version of Anki on Windows 7 when installed with Pip with the modified Python 3.9 installation.

* Update CONTRIBUTORS

CI req: Add myself to the contributor list
@abdnh abdnh mentioned this pull request Sep 26, 2022
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