- Issue created by @balintbrews
- 🇺🇸United States effulgentsia
Also the global CSS is more of a singleton use case
Is it? Or perhaps someone might want multiple, each representing a different look? For example, perhaps a university site where each department is branded a bit differently?
I think our initial implementation can be based on having just one, but I think conceptually having a config entity type for this that could in principle allow multiple config entities of that type would be good.
We need to come up with a name for this config entity type.
Also, would it make sense for the same entity to have any other property besides the CSS? For example, another global or semi-global thing might be a javascript import map (mapping bare package names of dependencies that the JS components might want to use, like perhaps components from Radix UI, to CDN assets, for example perhaps generated with https://jspm.org/). But I wonder if that belongs in the same entity as the global CSS (are the two each aspects of the same higher level concept like a theme or design system) or if global CSS and import maps belong in different entities / entity types (are they aspects of different concepts such as global CSS being an aspect of a theme whereas import map is an aspect of a component library)?
- 🇬🇧United Kingdom longwave UK
Should this be part of the page template entity, given we currently have one of those per theme?
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
#5++
That addresses this concern by @effulgentsia:
I think our initial implementation can be based on having just one, but I think conceptually having a config entity type for this that could in principle allow multiple config entities of that type would be good.
But
Also, would it make sense for the same entity to have any other property besides the CSS? For example, another global or semi-global thing might be a javascript import map (mapping bare package names of dependencies that the JS components might want to use, […]
is a very different beast. I'd rather keep the scope restricted to just CSS.
If we can do that, then I think @longwave's idea works.
- 🇫🇮Finland lauriii Finland
Singleton would be fine for the 0.3.0 release but I agree that we may want to put some consideration to the long term need. We may want multiple global stylesheets in future but I'm not sure how much we need to account for that now. If we start with a singleton (i.e. one stylesheet per theme), wouldn't it be pretty straightforward to migrate to whatever format we decide in future?
I'm not sure that the page template is the right config to tie this with. In future, I'd imagine we would tie the global CSS to the style guide once that becomes available. Given that the style guide is not fully defined yet, it might be hard to take everything into account now.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
#7 doesn't really answer my "different beast" remark. 😅 @lauriii told me in DM that we definitely need that eventually. Fine!
I don't know enough about JS import maps yet, nor why we need them (presumably for JS components, i.e. many JS components might all need the same import map?). And perhaps we don't know enough about it in general yet, given ?
Global CSS is comparatively simpler, so let's restrict this to just that. 🙏
wouldn't it be pretty straightforward to migrate to whatever format we decide in future?
Absolutely! :) Not even migrate, but update :D (Update path tests for config entities are a solved problem 😃)
- 🇺🇸United States effulgentsia
Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.
- 🇺🇸United States effulgentsia
Having thought about the import map problem space a bit more, I don't think it makes sense as global or per-theme config, but rather as something that gets merged dynamically based on information that's in each code component. I opened ✨ Allow code components to import from npm packages Active with more detail about that, but the point is, it means there's no other configuration that we currently know of that would be "per style guide" (see #7) other than the global CSS.
#7 says we probably don't want to combine this with the page template entity. I'm not completely clear on why we would or wouldn't want to do that, so let's go along with @lauriii's intuition on this.
Which means, we're talking about a new config entity type whose only property is the global CSS. So for naming, I guess we should go with
*.global_css.yml
? Possibly in the future we might figure out the details of a*.style_guide.yml
and would need to update from the former to the latter, but #8 indicates that that's okay? - 🇬🇧United Kingdom longwave UK
> We may want multiple global stylesheets in future
I don't get why we are saying this here, but previously we stated that we would only ever have one page template per theme. To me a global stylesheet ties in with a global page template. Back then I could see that I might want multiple page templates - some "special" pages might want to opt in to an entirely different template via some mechanism - but we ruled that out, so why aren't we doing the same for global CSS?
Until the way ahead and the future requirements are clearer, I still think the simplest option is to add this as an additional property to the page template config entity. As stated in #8 we can always migrate CSS from the page template to somewhere else. It's easier to add a single property to an existing config entity and move it later, than it is to define a whole new config entity and refactor/remove the whole thing later.
- 🇫🇮Finland lauriii Finland
I don't get why we are saying this here, but previously we stated that we would only ever have one page template per theme.
I don't think this assumption is true – at least it's not an assumption we should rely on. What might change this in future is having to implement support for variants. I don't know how we'd implement these technically, but one possible option may be to allow multiple page templates in a theme with some negotiation on which page template should be resolved to a given route. I don't know how fundamental this assumption is currently but if it is, maybe we need to try to design the system for the variants soon so that we can plan for what needs to change.
To me a global stylesheet ties in with a global page template. Back then I could see that I might want multiple page templates - some "special" pages might want to opt in to an entirely different template via some mechanism - but we ruled that out, so why aren't we doing the same for global CSS?
Global CSS in it's most basic form is different from this though. There needs to be some place where it is possible to store design tokens that are true across the whole system. On top of that, we may need a separate stylesheet in future that is tied into a page template as an additional form of customization but it doesn't seem like a mutually exclusive concept.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
@effulgentsia at #10: great, I inferred the same ~8 hours ago at #3500761-7: [SPIKE] Allow code components to import from npm packages → (i.e. that this metadata/dependency information belongs in the config entity that ✨ Config entity for storing code components Active is introducing).
I wholeheartedly agree with @longwave in #11.
@lauriii in #12: there's not a single word in the 64 XB product requirements about variants for page templates. Only for content type templates, the concept of "variants" is stated. Route-specific choices should be achieved using requirement
41. Conditional display of components
per the requirements. If your thinking has evolved, can you please add a new requirement there? 🙏 I don't see how any one of us could possibly have known your intent to support "page template variants".(Not that I think that supporting
PageTemplate
variants would be complicated: it'd only require changing\Drupal\experience_builder\EventSubscriber\RenderEventsSubscriber::onSelectPageDisplayVariant()
. But whether variants for some subset of routes should be to the "master variant" for a theme (completely different component trees for all regions and/or completely different global CSS), or whether it'd just be targeted overrides (completely different component trees for some regions and/or additional global CSS, on top of what the "master variant" contains) impacts everything here a lot. We need to deliver 🌱 [Meta] Plan for code components Active by the end of February, this issue is not the time nor place to start exploring potential future things.There needs to be some place where it is possible to store design tokens that are true across the whole system.
Global CSS is an orthogonal concern to design tokens. Design tokens is not being worked on for 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active . Design tokens may be exposed through CSS, but that's an implementation detail for later. Very little is known/defined for how "design tokens" will appear in the UI and how they impact other XB functionality. Let's stay focused on the task at hand 🙏 The issue summary here is about the ability to store (not even load!) CSS rulesets that apply to >=1 in-browser code components.
AFAICT the question that truly needs answering here for this issue's scope is: If the answer is:
- "theme-agnostic": then we cannot store the global CSS for in-browser code components in the
PageTemplate
config entity, because aPageTemplate
config entity is necessarily theme-specific. - "theme-coupled": then we can store the global CSS for in-browser code components in the
PageTemplate
config entity.
- "theme-agnostic": then we cannot store the global CSS for in-browser code components in the
- 🇺🇸United States effulgentsia
Are in-browser code components supposed to be theme-agnostic or should they only be available within a particular theme?
I don't even think that that's relevant. CSS for a code component will live in the code component's config entity. So the question in this issue is just whether the global CSS should be per-theme or not, and I think that's independent of whether code components are per-theme or not. @lauriii in #7 already said that global CSS is per-theme, which makes sense to me.
One problem with tying the global CSS to the page template, is then does that mean that someone who doesn't check the Use-XB-for-regions checkbox also doesn't get global CSS? Or is that not an issue anymore with 📌 PageTemplate: allow configuring which regions are exposed Active , since now we can change our logic to always create the page template and just not opt it into any regions until the user opts those regions in?
Also, in the future we may well want to have >1 page template for the same theme sharing the same global CSS, but if we don't mind moving the global CSS to a different config entity type at that point in time, when hopefully we'll have more clarity about which config entity type to move it into, then I think it's fine to put it in the page template for now as long as the paragraph above has an answer.
- 🇬🇧United Kingdom longwave UK
@wim leers I discussed this a bit on standup with @lauriii. We came to the conclusion that "global CSS" (which perhaps needs a different name) should be stored separately from page templates. Initially there will be a 1:1 mapping of global CSS to themes, the same as page templates, but perhaps in the future you might want multiple CSS files, in which case we would need to extend that out. Also, to the site owner, editing global CSS will be done separately from editing the page templates, so it seems to make sense to keep the concepts separate on the back end as well. As #14 states you might want to use global CSS without enabling page templates for the current theme; the global CSS will be applied whether or not a page template is currently in use.
- 🇺🇸United States effulgentsia
"global CSS" (which perhaps needs a different name)
Yep, naming is hard. "global" here refers to "not tied to a single component". I think "global CSS" is a common term in front-end development, but "global" here isn't meant to imply back-end globality (like there only being one for the entire site, regardless of theme, etc.).
One possibility is we name the new config entity type "style_guide"? Even though we don't know exactly what this concept will entail yet, but we can start it off with a
css
property and find out later what other properties to add to it? And we start off with one Style Guide entity per theme, and perhaps later there'll be a product requirement to allow >1 style guide per theme? - 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
@longwave in #15:
editing global CSS will be done separately from editing the page templates, so it seems to make sense to keep the concepts separate on the back end as well.
This is the strongest argument to make it separate.
the global CSS will be applied whether or not a page template is currently in use.
That actually answers my question from #13: it should be theme-agnostic.
But it contradicts @effulgentsia's
@lauriii in #7 already said that global CSS is per-theme, which makes sense to me.
(I do not read that in #7, by the way. AFAICT "one stylesheet per theme" was merely in response to what @longwave was proposing in #5.
@effulgentsia in #14:
So the question in this issue is just whether the global CSS should be per-theme or not, and I think that's independent of whether code components are per-theme or not.
That's not independent though: a
JavascriptComponent
config entity needs to have all global CSS it relies upon to be loaded. That means it must depend on the config entity storing/providing that global CSS.So if the global CSS is stored in the
PageTemplate
config entity, then necessarily theJavascriptComponent
must depend on that page template's theme.If the global CSS is not stored in the
PageTemplate
config entity, then it must live in another config entity, and then theJavaScriptComponent
config entity must explicitly depend on that.AFAICT that means this issue must introduce a new
SharedCss
config entity type (because "global" is a misnomer as @longwave points out in #15). But … how should it be identified? Perhaps we start with a "singleton" and default todefault
as the ID of the soleGlobalCss
config entity?Do we agree? If so, we should overhaul the issue summary to reflect the actually intended scope rather than the original scope. I already updated the title to match my current understanding — feel free to change it again if I misunderstood! 😅
Future questions this raises, which are all out-of-scope for this issue:
- How does a
JavascriptComponent
specify whichSharedCss
it relies upon, and how do we expose that in the UI? - How do we guarantee that it works correctly across all themes? (AFAICT we don't/we can't/it's up to the JS component author?)
- If/when the
PageTemplate
needs some CSS in the future, it too could just start depending on aSharedCss
config entity.
- How does a
- 🇳🇱Netherlands balintbrews Amsterdam, NL
I'm gravitating towards a more abstract name, and I keep thinking how the concept is similar to what Drupal already calls libraries, and have been wondering if I should propose naming the config entity in-browser code library.
It would store JS/CSS source + compiled. (Similar to ✨ Config entity for storing code components Active , but without props and slots, and without ✨ Create a ComponentSource plugin for JS components Active being concerned about it.)
Then the "style guide" ( #16 ✨ Storage for global CSS Active ) / "shared CSS" ( #17 ✨ Storage for global CSS Active ) would be one way of consuming the capability of creating in-browser code libraries, but we might come across others. Thoughts?
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
#16 — but @lauriii and @longwave in #15 concluded , which implies theme-agnosticness?
#16 + #18: I want to keep 'style guide' out of this conversation. 🙏Nowhere near enough is known about it. We shouldn't risk missing 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active for something we don't know what it'll look like exactly yet, plus which we can easily refactor towards in the future.
#18:
- What would "compiled CSS" be?
- Can you provide an example of how two "code component" config entities depend on the same JS?
- I see no plans in the GIFs/designs at 🌱 [Meta] Plan for code components Active for JS that is shared across code components?
- 🇳🇱Netherlands balintbrews Amsterdam, NL
#16 And we start off with one Style Guide entity per theme, […] — but @lauriii and @longwave in #15 concluded the global CSS will be applied whether or not a page template is currently in use., which implies theme-agnosticness?
I feel like we can go back and forth whether the global CSS is meant to be theme agnostic. If we want to architect the perfect system, the answer will be yes and no. 🙂 That's why I'm proposing to work out a more generic storage mechanism, then we can do a simple layer on top with a simple use case as a start, i.e. the 1:1 mapping that was already mentioned.
#16 + #18: I want to keep 'style guide' out of this conversation. 🙏Nowhere near enough is known about it. We shouldn't risk missing #3455753: Milestone 0.3.0: Early preview for something we don't know what it'll look like exactly yet, plus which we can easily refactor towards in the future.
I think #16 proposed it merely as a name, nothing more, and I simply mentioned the already proposed names in #18.
#19 ✨ Storage for global CSS Active :
- In the case of our Tailwind CSS v4 support, it would be the Tailwind-generated CSS resets + the transpiled version of the source CSS. In other (potential, future) cases it would simply be the transpiled version of the source CSS (think of autoprefixing and syntax lowering).
- In case of implementing global CSS, it would be assumed that it's added on all pages. We don't need to define explicit dependencies.
- No use case so far. But we may need it after we have ✨ Allow code components to import from npm packages Active .
- 🇫🇮Finland lauriii Finland
there's not a single word in the 64 XB product requirements about variants for page templates. Only for content type templates, the concept of "variants" is stated. Route-specific choices should be achieved using requirement 41. Conditional display of components per the requirements. If your thinking has evolved, can you please add a new requirement there?
Clarified this in the requirements 👍 I'm glad this doesn't something that would be particularly complicated for us to introduce later. I'm also glad that we discussed about the requirement now that we were about to introduce something new that potentially may have impacted implementing this in future. 😊
#16 And we start off with one Style Guide entity per theme, […] — but @lauriii and @longwave in #15 concluded the global CSS will be applied whether or not a page template is currently in use., which implies theme-agnosticness?
The CSS should be tied to a specific theme. This would be used essentially for the same purposes as what the
libraries
key in themes.*.info.yml
is used (as mentioned by @balintbrews in #18). If someone wants to reuse the same CSS in another theme, depending on their requirements, they could either set up a base theme with the common CSS or they could just copy the CSS to the other theme.How does a JavascriptComponent specify which SharedCss it relies upon, and how do we expose that in the UI?
There's no explicit dependency to the global CSS in Drupal today. This assumption introduces many problems but I think we should keep it that way going forward because otherwise we'd be essentially introducing a 1:1 coupling of the theme and its components.
How do we guarantee that it works correctly across all themes? (AFAICT we don't/we can't/it's up to the JS component author?)
This needs to be taken into account in the way that components are build. E.g. they need to avoid hard dependency on the global CSS (e.g. providing default values for CSS variables when they do not exist). This is a hard problem that for example Tailwind tries to solve.
Not everyone cares about this. Many frontend developers are fine with introducing explicit dependencies from their components to their global CSS via CSS variables and rules. We should make it clear in documentation that doing this will make it harder for them to adopt external components. Unless you aim to repurpose your components or import external components, there's nothing wrong in that.
- 🇫🇮Finland lauriii Finland
I feel like we can go back and forth whether the global CSS is meant to be theme agnostic. If we want to architect the perfect system, the answer will be yes and no. 🙂 That's why I'm proposing to work out a more generic storage mechanism, then we can do a simple layer on top with a simple use case as a start, i.e. the 1:1 mapping that was already mentioned.
Had a brief conversation with @balintbrews to make sure we're aligned on the solution. We're aligned that having a single CSS file per theme for shared CSS within that theme is enough to capture most of the value and enable the Preact + Tailwind v4 solution that the team has been architecting. 👍
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
@balintbrews in #20: Thanks!
++
- 👍
- 👎 Because of 2 reasons:
- @lauriii states pretty definitively in #21 states again that it's theme-specific global CSS — so at minimum it'd require a dependency on a theme then. More about that below.
- That won't work for requirement
14. config management
, because I could create code component A that assumes some global CSS on one site, and code component B in another environment that assumes other global CSS. When creating new config entities for XB, we must always do so in a way that requirement 14 is supported. That requires explicit dependencies. It almost makes me think that the global CSS should be stored as
- 👍
@lauriii in #21:
Thanks for adding requirement45.1 Page template variants
😊The CSS should be tied to a specific theme.
If the "global CSS" is tied to a specific theme, then that means that the code component being built (using component-specific JS, component-specific CSS and theme-specific "global CSS") itself depends on that theme. This appears to be confirmed by:
If someone wants to reuse the same CSS in another theme, depending on their requirements, they could either set up a base theme with the common CSS or they could just copy the CSS to the other theme.
Great! This is an important clarification! 🥳 And it actually means that @longwave's original proposal from #5 ("just store global CSS in the
PageTemplate
config entity for a theme") is relevant again, and the lowest level of effort. But, it'd result in an auto-save entry that looks weird — at least until 📌 The pending changes API endpoint should list individual regions for global template changes Active allows seeing not "the (theme's) page template" as a unit of change to review/publish, but "a region in the theme's page template". From that POV, it'd then also make sense to expose "the theme's page template's global CSS" as its own line item in the "Review changes" UI.However, the "theme-specific global CSS" should be prepared to support the new
45.1 Page template variants
, and the global CSS is per theme, NOT per theme's page template variant. Given that, it follows that we should introduce a new config entity. Or … we just choose to accept that we'll have to lift it out of thePageTemplate
config entity at the future point where we work on45.1
.There's no explicit dependency to the global CSS in Drupal today. This assumption introduces many problems but I think we should keep it that way going forward because otherwise we'd be essentially introducing a 1:1 coupling of the theme and its components.
(Emphasis mine.)
Exactly, that coupling is a dependency which config entities must describe to not violate requirement14. config management
, but which you're stating we must somehow not do while still keeping things working reliably! 😬How does this not conflict with your paragraph just before it? 🤔
💡 We could default to the
JavascriptComponent
config entity having an explicit dependency on the theme's global/shared CSS. That'd prevent problems for14. config management
. We could then still make it possible to sever those ties by providing an explicit or or button, which would literally just omit that one explicit dependency. That AFAICT gets you everything you want:- maximum reliability (the default: dependency present)
- maximum reuse (an explicit action that omits the dependency, which it must be anyway, because transplanting components from one site to another requires a subset of site A's config to be imported into site B — and this is all very close to requirement
1.2 Design system (foundations)
's "shipped as a package" statement!)
they need to avoid hard dependency on the global CSS
Will the in-browser code component creation UX toggling the preview having the global CSS applied vs not, to make it possible to verify the fallback nature?
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
I updated the issue summary + title based on my current understanding.
In doing so, I tried to unify hard requirements (e.g.
14. config management
support) with future intentions and ambitions (45.1 page template variants
,26. Styleguide
, ✨ Allow code components to import from npm packages Active …).From that POV, I think @balintbrews' proposal at #18 for the concept of an in-browser code library makes sense. That'd be conceptually similar to https://git.drupalcode.org/project/component_library/-/blob/1.0.x/src/En....
Any final thoughts before I begin implementing this? :)
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
@balintbrews' feedback about "compiled CSS/JS" has been incorporated.
- 🇳🇱Netherlands balintbrews Amsterdam, NL
Thank you, Wim, for distilling all those comments into this solid proposal. 👏🏻
I think this will achieve what we need for 🌱 [Meta] Plan for code components Active . While I'm quite comfortable with what's being proposed, it definitely wouldn't hurt to get others to review it as well.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Thinking this through on a run, I realized there's 3 things we've missed:
- major: sibling internal HTTP API issue for this new config entity, similar to
✨
HTTP API for code component config entities
Active
— see
#3499931-6: [PP-1] HTTP API for code component config entities →
.
(EDIT: I see that this was already somewhat accounted for by
Consider how this may reuse the work done in ✨ Config entity for storing code components Active and ✨ HTTP API for code component config entities Active .
— but it would likely be simpler to first tackle just the config entity here and then tackle all internal HTTP API needs for both config entity types in ✨ Auto-save code components Active ) - major: sibling issue for this new config entity, similar to ✨ Auto-save code components Active (see point 1)
- minor:
dependencies
— asset libraries may need to depend on other asset libraries: not a problem for the scope of this issue; but we'll likely need that later
- major: sibling internal HTTP API issue for this new config entity, similar to
✨
HTTP API for code component config entities
Active
— see
#3499931-6: [PP-1] HTTP API for code component config entities →
.
- 🇳🇱Netherlands balintbrews Amsterdam, NL
#28 ✨ Storage for global CSS Active :
- My only suggestion/request would be to prioritize landing ✨ HTTP API for code component config entities Active . If adding support for the config entity defined in this issue in ✨ HTTP API for code component config entities Active would delay its completion, I suggest splitting that part into a separate issue — or perhaps expanding the scope of this one to handle it. This way, we won't hold up other issues.
- It might be worth waiting until the solutioning work is completed for ✨ Auto-save code components Active , as it will then be easier to create the sibling issue.
- Yes, let's do that later, I'm not sure this will be needed for 🌱 [Meta] Plan for code components Active . 🙂
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Just chiming in here to note that our 'compiled JS' configuration will likely need to maintain a set of externals that are common with the built code of XB
i.e. we know XB imports react and react-dom. If any of these code components also need them, then we need to make sure that neither the XB built code OR the compiled_js in this asset library includes the react or react-dom code base. Otherwise they will be referencing _different_ instances of React and we will end up with all sorts of edge cases.
Both of them will need to use the external rollup option in the vite config AND we'll need to make use of an import map.
Alex's comments indicate he's already across this, but I just want to make it explicit.