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

fix: auto switch theme behavior & footer theme indicator #4677

Merged
merged 2 commits into from
Oct 2, 2023

Conversation

sanidhyas3s
Copy link
Contributor

  • Changing manually to a preset or custom theme now turns auto switch theme mode off with a notification. And now the auto switch mode does override the custom theme as well (statement in settings also updated), priority simply given to the last way theme was modified.

  • Fixes Autoswitch theme causing the footer theme indicator to show wrong theme #4659, that is the footer theme is now correctly displayed with auto switch themes as well.

@monkeytypegeorge monkeytypegeorge added the frontend User interface or web stuff label Sep 28, 2023
@monkeytypegeorge
Copy link
Collaborator

Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. https://github.com/monkeytypegame/monkeytype/actions/runs/6342827952

@monkeytypegeorge
Copy link
Collaborator

Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. https://github.com/monkeytypegame/monkeytype/actions/runs/6343959925

@monkeytypegeorge
Copy link
Collaborator

Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. https://github.com/monkeytypegame/monkeytype/actions/runs/6344000764

@monkeytypegeorge
Copy link
Collaborator

Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. https://github.com/monkeytypegame/monkeytype/actions/runs/6344247626

Comment on lines 1330 to 1335
export function setAutoSwitchThemeOff(): void {
if (!config.autoSwitchTheme) return;
config.autoSwitchTheme = false;
saveToLocalStorage("autoSwitchTheme", undefined);
ConfigEvent.dispatch("autoSwitchTheme", config.autoSwitchTheme);
Notifications.add("Auto switch theme disabled", 0);
return;
}

Copy link
Member

Choose a reason for hiding this comment

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

Why add this? Why not just call setAutoSwitchTheme(false)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using that was causing RangeError: Maximum call stack size exceeded causing it to crash, can't really figure out why but maybe it somehow ends up recursively calling itself?

Copy link
Member

Choose a reason for hiding this comment

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

It was looping because you are setting autoswitchtheme to off when reacting to autoswitchtheme being changed to off. I added a check where its only set to off if its already on, and its all good.

Changing manually to a preset or custom theme now
turns auto switch theme mode off with a notification.
And now the auto switch mode does override the custom
theme as well (statement in settings also updated) if it
is the later one set.
Fixes monkeytypegame#4659, that is the footer theme is now correctly
displayed with auto switch themes as well.
@Miodec Miodec merged commit a7c0eb5 into monkeytypegame:master Oct 2, 2023
7 checks passed
Miodec added a commit that referenced this pull request Oct 2, 2023
…, miodec) (#4677)

* fix: auto switch theme behavior & footer indicator

Changing manually to a preset or custom theme now
turns auto switch theme mode off with a notification.
And now the auto switch mode does override the custom
theme as well (statement in settings also updated) if it
is the later one set.
Fixes #4659, that is the footer theme is now correctly
displayed with auto switch themes as well.

* removed unnecessary function

---------

Co-authored-by: Miodec <jack@monkeytype.com>
Miodec added a commit that referenced this pull request Oct 2, 2023
* Add files via upload

* Delete frontend/static/sound/error/triangle.wav

* Delete frontend/static/sound/error/damage.wav

* modified error sound code to support multiple sound options and added two error sound alternatives

* added compatability for previous users of the error sound, converts legacy true/false config values to 1 or off

* fixed opiton names and values in commandline

* fix: auto switch theme behavior & footer theme indicator (#4677)

* fix: auto switch theme behavior & footer indicator

Changing manually to a preset or custom theme now
turns auto switch theme mode off with a notification.
And now the auto switch mode does override the custom
theme as well (statement in settings also updated) if it
is the later one set.
Fixes #4659, that is the footer theme is now correctly
displayed with auto switch themes as well.

* removed unnecessary function

---------

Co-authored-by: Miodec <jack@monkeytype.com>

* fixed off config value

* moved compatibilty code to replaceLegacyValues

---------

Co-authored-by: Sanidhya Singh <sanidhyas3s@gmail.com>
Co-authored-by: Miodec <jack@monkeytype.com>
@sanidhyas3s
Copy link
Contributor Author

@Miodec You removed the notification as well with the function, shall I add that in a new PR?

@Miodec
Copy link
Member

Miodec commented Oct 2, 2023

@Miodec You removed the notification as well with the function, shall I add that in a new PR?

Yeah, sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend User interface or web stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Autoswitch theme causing the footer theme indicator to show wrong theme
3 participants