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

feat: class directive on <svelte:body> #6928

Closed
wants to merge 9 commits into from

Conversation

andirady
Copy link

@andirady andirady commented Nov 14, 2021

Closes #3105

Supports class directive on <svelte:body>. The following works:

<script>
  let one = false;
</script>

<button on:click={e => one = !one}>Toggle one</button>

<svelte:body class:one />

<style>
  :global(.one) {
    background: black;
  }
</style>

Note that the class is global. Without :global the compiler will produce a warning.

P/S: Codes are copied from src\compiler\compile\render_dom\wrappers\Element\index.ts 😋

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with [feat], [fix], [chore], or [docs].
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

Copy link

@malihakkak malihakkak left a comment

Choose a reason for hiding this comment

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

Gg

@Conduitry
Copy link
Member

As I mentioned on #3105, I'm worried about doing this without it also working in SSR, and we would need to agree on an API for that. It should probably look like stringified attributes - rather than just a list of class names - so that it can be expanded to other features later.

That these classes would not be scoped makes more sense than them being scoped, I think, but needing :global() still feels confusing and it would help to document it. Also, if someone specifically styled the selector body.foo, how should that be treated? I don't have a clear answer for that one yet.

@Conduitry Conduitry marked this pull request as draft December 13, 2021 15:54
@ghost
Copy link

ghost commented Jan 27, 2022

Is there any movement on this request?

issue #3105 was created 3 years ago
but still no results :(

@el-schneider
Copy link

I would also be interested in using this with SSR

@andirady
Copy link
Author

I would also be interested in using this with SSR

Since this is only adding class to the body element, I don't think SSR support is really necessary. The script can add the class once it's loaded.

@el-schneider
Copy link

el-schneider commented Feb 20, 2022

Since this is only adding class to the body element, I don't think SSR support is really necessary. The script can add the class once it's loaded.

In my specific case it was a static page that didn't hydrate, which is something sveltekit supports and is a not too uncommon use-case. In this case it wouldn't work an still be handy.

@benmccann benmccann changed the title [feat] allow to use class directive on <svelte:body> (#3105) feat: class directive on <svelte:body> Mar 14, 2023
@dummdidumm dummdidumm added this to the one day milestone Mar 24, 2023
@vercel
Copy link

vercel bot commented May 24, 2023

@andirady is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

@Stephen10121
Copy link

Can someone please authorize this?

@Rich-Harris
Copy link
Member

Closing Svelte 4 PRs as stale — thank you (in the case of this issue, it really is something that should happen programmatically, so that developers are forced to deal with a) the lack of SSR support and b) the possibility of conflicts between components)

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.

Change body class via <svelte:body />
7 participants