UWCS / uwcs-dextre

The University of Warwick Computing Society website for the 2020-21 academic year.
https://uwcs.co.uk
MIT License
1 stars 2 forks source link

Improve user flow for toggling dark mode/auto theme #47

Closed joshdavies14 closed 2 years ago

joshdavies14 commented 2 years ago

We have two toggles next to each other - while they do different things (one toggles between light/dark mode, the other toggles auto-chaning theme based on system on/off), can we change what we show when/the flow for disabling auto theme?

image

Possible suggestions:

  1. Only show "night mode" toggle if auto-theme is disabled
  2. Change to something like we have in Chrome/Edge/Firefox settings - three options "System default/Light/Dark" where system default is auto enabled like how it currently works.
  3. Show modal if session attributes don't exist to ask user preference? This wouldn't remove the need for the toggles though, as we need to consider what if the user wants to change their preference while an active "session" exists.

We can't just move these toggles to account settings, as not everyone who uses the site has an account. Option 2 above is my preferred option right now - option 1 is a quick fix but still leaves us with two toggles. I don't really like option 3 right now - it's more work to implement compared to both other options, and doesn't remove the current issue. Especially as this website is supposed to be repleaced soon it's probably more work for now than is necessary, in my opinion.

Levelent commented 2 years ago

When the user has a system default of dark theme and has 'auto colour theme' enabled, we can do this:

  1. Disable 'auto colour theme' on a page (automatically enabling the night mode toggle)
  2. Move to another page
  3. Disable night mode

Then, the auto colour theme re-enables. Assuming this is unintended behaviour?

joshdavies14 commented 2 years ago

Yeah auto should stay off in that case - https://github.com/UWCS/uwcs-dextre/blob/master/accounts/views.py#L180 should be wrapped in an if statement in this case

Levelent commented 2 years ago

Additionally, repeatedly toggling night mode while the 'auto colour theme' is disabled will lead to the site remaining on night mode regardless of what the toggle is set to

joshdavies14 commented 2 years ago

Yeah auto should stay off in that case - https://github.com/UWCS/uwcs-dextre/blob/master/accounts/views.py#L180 should be wrapped in an if statement in this case

Linked to this I believe, unless when I try to replicate I am just hitting this issue

joshdavies14 commented 2 years ago

Additionally, repeatedly toggling night mode while the 'auto colour theme' is disabled will lead to the site remaining on night mode regardless of what the toggle is set to

Actually, I can't replicate in Firefox but can in Edge, where the issue is above where auto does not stay off if manual dark mode is turned off. Surprisingly the auto theme doesn't seem to re-trigger in Firefox so who knows what's going on.