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

Add scoped class feature #30

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Add scoped class feature #30

wants to merge 25 commits into from

Conversation

backspace
Copy link
Contributor

@backspace backspace commented Apr 1, 2024

On hold: we’ve decided this isn’t worth the increased complexity, it can be accomplished with :global and a unique class name instead. If you would find this feature useful, please let us know in the comments.


This adds a __GLIMMER_SCOPED_CSS_CLASS that facilitates style scoping even when working with code we don’t control that generates elements dynamically, such as Ember Power Select.

See here for the AST for the test component.

There are some irksome formatting-only changes, I changed it to use the same Prettier branch as cardstack/boxel because format-on-save was causing test-app/app/components/uses-scoped-classes.gjs to become empty 😖

Copy link

@lukemelia lukemelia left a comment

Choose a reason for hiding this comment

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

Looking good

Copy link

@lukemelia lukemelia left a comment

Choose a reason for hiding this comment

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

Nice work. You can really climb an (abstract syntax) tree!

@ef4
Copy link
Contributor

ef4 commented Apr 10, 2024

I would have thought that :global() was already a good enough escape hatch for this.

It's true that using global you have to pick a name to use, but that might be a positive, given that the name will be used far from its definition and if it's randomized there's no way to trace it back to its source. (This is not a concern for the rest of our scoped attributes since there's exactly one place that could be responsible for each one, since they're all local.)

@tintinthong
Copy link

tintinthong commented Apr 15, 2024

I would have thought that :global() was already a good enough escape hatch for this.

:global could definitely work as an escapte hatch. But, if I used :global id (as u said, without choosing a name) I will get clashes in CSS. Something like below,"5:17:06 PM" is scoped using this feature but "Some glob..." is styled using a global that clashes.

Screenshot 2024-04-15 at 20 48 42

It's true that using global you have to pick a name to use, but that might be a positive, given that the name will be used far from its definition and if it's randomized there's no way to trace it back to its source. (This is not a concern for the rest of our scoped attributes since there's exactly one place that could be responsible for each one, since they're all local.)

@ef4 Do you mean its confusing to have the same __GLIMMER_SCOPED_CSS_CLASS name in the code base, although in the generated html, two different templates that use __GLIMMER_SCOPED_CSS_CLASS generates a different class name .scopedcss-xyz-abc and .scopedcss-qwe-rty ?? This api not a good idea?

@ef4
Copy link
Contributor

ef4 commented Apr 15, 2024

Do you mean its confusing to have the same __GLIMMER_SCOPED_CSS_CLASS name in the code base,

No, I was referring to the generated class name. Using this PR, if you debug the far-away element it will say something like scopedcss-f61ca3e17a-5096f06db6 and you'll have no way of connecting it back to the component that's responsible for it.

Whereas if you did

<template>
  <style>
    :global(.my-special-component) {
        color: red;
    }
  </style>
  <PowerSelect @class="my-special-component" />
</template>

The element would say my-special-component on it and you could search for that name to see where it came from.

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.

None yet

4 participants