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

LightSwitch Service Improvements #737

Closed
lairdresearch opened this issue Dec 23, 2022 · 7 comments
Closed

LightSwitch Service Improvements #737

lairdresearch opened this issue Dec 23, 2022 · 7 comments
Assignees
Labels
feature request Request a feature or introduce and update to the project.

Comments

@lairdresearch
Copy link

Describe what feature you'd like. Pseudo-code, mockups, or screenshots of similar solutions are encouraged!

As lightswitch is currently configured, it is assumed it will be present on every page and able to trigger the correct mode. In some cases, developers may not have lightswitch on each page or have a separate set of functions to control dark mode. In particular, LightSwitch holds its state in two stores - while these could be imported separately & managed, it would be better to have a utility function which manages that capability. Otherwise, developers will end up recreating the same code inside of LightSwitch to handle the stores themselves -- it is better to build a function with a standard interface to manage this.

I propose that the two functions currently in LightSwitch: setPrefersDarkScheme() and setElemHtmlClass(), be extracted an put into a separate file. This file can be imported into LightSwitch and used as-is. However, they could also be separately imported at other parts of the app and used to manage Dark mode without requiring LightSwitch to be present. So -- no new code, just separate it out into a different file for independant use.

Probably makes sense to change the names of the functions to something more descriptive as well.

What type of pull request would this be?

Enhancement

Any links to similar examples or other references we should review?

No response

@lairdresearch lairdresearch added the feature request Request a feature or introduce and update to the project. label Dec 23, 2022
@endigo9740
Copy link
Contributor

endigo9740 commented Dec 23, 2022

Thanks @lairdresearch. We'll aim to take a look at this soon! Of course PRs are always welcome if you or anyone else can get to this sooner!

@endigo9740 endigo9740 added the help wanted Extra attention is needed label Dec 23, 2022
@endigo9740 endigo9740 added this to the Future/Whenever milestone Jan 2, 2023
@endigo9740
Copy link
Contributor

I'm including this to visualize how I'd like this to work. This is just a mock, not a complete solution, but should help illustrate the goal.

Screen Shot 2023-01-16 at 11 24 29 AM

  • Left: any UI as the "frontend" for the toggling
  • Right: query the OS preferred option
  • Right: get/set the user preference
  • Middle: the service that handles all the business logic between all this

@endigo9740
Copy link
Contributor

@Sarenor
Copy link
Contributor

Sarenor commented Jan 21, 2023

I am not so sure about this, the UK law and the German law is pretty much exactly the same here, so I'll quote the UK law for it:

There is an exception to the requirement to provide information about cookies and obtain consent where the use of the cookie is:

(a) for the sole purpose of carrying out the transmission of a communication over an electronic communications network; or

(b) where such storage or access is strictly necessary for the provision of an information society service requested by the subscriber or user.

I would argue that both the theme and the dark mode storage are requested services as soon as the user interacted with it.
So to be 100% safe we "just" have to make sure that a Cookie/LocalStore is only set if the user manually updates their preferences.

That's just my take and I'm not a lawyer, etc, you know the drill ;)

@lairdresearch
Copy link
Author

lairdresearch commented Jan 21, 2023

this is actually an opportunity to add a nice dev feature -- since the lightswitch is stored in localstorage and this is viewed as a concern, then having that middleware available is the opportunity to ask the user if they consent to localstorage -- permission which could be used in many other places. This makes it an ideal shared piece of information.

So maybe the process for dark mode is:

  1. In the base layout.svelte page, include a component which sets the dark/light mode (this is Chris' Dark Mode service). It would read the system preferences for dark mode and set accordingly as a default.
  2. There is a separate persistence component - the first time something tries to store info there (eg. the dark mode service), it would pop up to ask the user if storing stuff on computer is fine. If true, it returns a localstorageStore to the dark mode service to store the default, if false, just a normal store (which obviously doesn't persist). However, it also would store the preference for GDPR (note that nothing being stored is also a signal to ask the user). Call this the privacy service
  3. Dark Mode service asks the Privacy Service for a store to hold the current value and if a localstorageStore is returned, a prior session's value could be read. In the future if it is updated with the light switch, then it would update the store -- but the store is configured (either as for the session or long term in localstorage) to respect privacy.

This would be a nice add-in for skeleton because I'm sure lots of designs need this information so you can solve it once and have it be a resource for the rest of the app.

Just a thought.

@endigo9740
Copy link
Contributor

endigo9740 commented Jan 30, 2023

This this post about SSR issues with the current iteration of the Light Switch:

Also, to review:
https://css-tricks.com/almanac/properties/c/color-scheme/

@endigo9740 endigo9740 changed the title Allow easier management of Dark Mode preferences by not requiring LightSwitch to be present on every page. LightSwitch Service Improvements Feb 3, 2023
@endigo9740 endigo9740 self-assigned this Feb 3, 2023
@endigo9740 endigo9740 removed the help wanted Extra attention is needed label Feb 3, 2023
@endigo9740
Copy link
Contributor

Solved as part of: #922

Closing this ticket. Please refer any further comments to the ticket linked above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Request a feature or introduce and update to the project.
Projects
None yet
Development

No branches or pull requests

3 participants