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

Is global the right way to go? #32

Open
mdpotter55 opened this issue May 15, 2021 · 2 comments
Open

Is global the right way to go? #32

mdpotter55 opened this issue May 15, 2021 · 2 comments
Labels
type/feature A new feature or functionality

Comments

@mdpotter55
Copy link

mdpotter55 commented May 15, 2021

This could be done with a wrapping element that does not affect the dom's structure beyond css. A mix of setProperty() and removeProperty() could be used to handle the theme transitions.

I do like your object storage for variable settings. A lot cleaner than what I was working on.

Possible benefits:
Global default variables could be defined in script to handle non-js systems. (rare to be sure)
-- global defaults also allow for partial rewrites of the variables. (i.e. no need to rewrite media breakpoints unless desired.)
Applying to a wrapping element allows for the nesting of variable settings. Perhaps modals look different.
Named wrappers could still use localStorage for defaults.
This allows for easier real-time changes with user-defined themes.
Property setters make the code cleaner since you don't have to build complex strings beyond your tiered variable names.

Listed below is a simple example. Everything in the slot would see the variable --txtColor as red.
display: contents; removes the wrapper from creating display issues (grids, etc).

<script>
	import { onMount } from 'svelte';
	let el;	
	onMount(() => {
		el.style.setProperty('--txtColor','red')
	});
	
</script>

<div bind:this={el} style="display: contents; ">
	<slot ></slot>
</div>
@josefaidt josefaidt added the type/feature A new feature or functionality label May 16, 2021
@josefaidt
Copy link
Owner

Hey @mdpotter55 👋 thanks for raising this, and glad to hear you're enjoying the library! We can definitely add the variable styles to the wrapper component

\cc @dysfunc thoughts?

@mdpotter55
Copy link
Author

mdpotter55 commented May 16, 2021

It is only a point of discussion. The more I write things down, the better I see them. My method has its own issues to consider.

There is another way:

  1. name a theme 'root' as a requirement (or it will default to the first).
  2. apply the 'root' theme variables to ':root' to set the default.
  3. All other themes are considered overlays and declared as classes.
  4. When a user picks a theme, apply the overlay class to html or body.
  5. This also allows an overlay to be used directly as a class instead of having to nest with a control.
  6. This method can be accomplished with zero controls or a visually empty one. (perhaps even a preprocessor)
  7. This method should also work much better with SSR & SEO and non-js (via svelteKit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature A new feature or functionality
Projects
None yet
Development

No branches or pull requests

2 participants