For MySQL itself, it's a bit more vague: https://launchpad.net/ubuntu/noble/+package/mysql-server but de-facto you still get 8.0 it looks like. However I wonder if they're going to switch that mid-release to 8.4 (supported until 2029 according to https://endoflife.date/mysql) as 8.0 goes EOL). So maybe it would be fine to require 8.4 according to #8 if ubuntu installs get updated anyway.
I don't see what's vague in that link. It looks clear cut that Ubuntu 24.04 is on MySQL 8.0. I don't think there's any precedent for Ubuntu to raise the minor version of MySQL (or any other significant package) within an LTS branch that has already been released. Based on all of their track record, I expect Ubuntu to security patch MySQL 8.0 for as long as 24.04 is supported (till 2029 minimum and beyond that for extended support). I think #8 would clearly dictate having Drupal 12 require MySQL 8.0, not 8.4. I don't know if we have consensus on #8, but I'm still in favor of that. Most people are not going to upgrade from Ubuntu 24.04 to 26.04 within the first year of Drupal 12 being out, and while it's true that those people can remain on Drupal 11, I don't think we gain enough from raising the MySQL minimum to 8.4 to be worth the tradeoff in holding back Ubuntu 24.04 users from upgrading to Drupal 12.
Oh! Well that seems like something that's both relatively easy to do and would reduce confusion for beta testers, so tagging it as a beta target, but not a beta blocker.
Closing this as outdated. The Undo and Redo buttons work, and if there are future designs to change their location, new issues can be opened for that then.
Untagging "stable blocker", because XB 1.0.0 does not require a public extensions API. That can come in future minor 1.x releases. The goal of XB 1.0.0 is to get it into people's hands for building and deploying real websites with it, and they don't need to extend the XB UI to do that.
Initially we were only planning to postpone Workspaces to a later 0.x version, but now we're postponing it till after 1.0.0, so re-titling this issue, and we may want to adjust the language in the MR as well.
Where Workspaces will become increasingly crucial is for editing individual nodes within XB, but that's also something we're postponing until after 1.0.0. 1.0.0 will just be for Pages and Content Templates.
I agree in principle that ADRs are valuable and that they're more valuable sooner than later, so I'll try to make time for this when I'm able to, but we're not going to block a stable release on getting this ADR published.
Removing "stable blocker" tag but we'll want to fix this as part of the Content Templates work, so setting the issue parent accordingly.
See #3503038-15: Refactor GeneratedFieldExplicitInputUxComponentSourceBase and FieldForComponentSuggester to need only SDC's ComponentMetadata, not SDC plugin instances → . I'm very much in favor of this issue, but we're trying to pare down the "must haves" for a stable 1.0.0 release in order to unblock a stable Drupal CMS 2.0 release as soon as possible.
I read #10, but I don't think this is a stable blocker. We'd be happy to merge this once it's ready though, so by untagging it I have no desire to slow this issue down.
For XB, the definition of stable is different than core's. We'll be adding @internal to almost everything and relaxing that in 1.x minor releases. The main goal of an XB stable release is to unblock a Drupal CMS 2.0 stable release, allowing people/agencies to build and deploy real sites with that. But XB 1.0.0 won't yet have a proper PHP API surface for contrib projects to perform more advanced integrations than what DCMS 2 exercises, except to the extent that issues like this one just happen to get done ahead of a 1.0.0 release.
Now that we're effectively in beta, changing route names requires a BC layer
I haven't reviewed the MR to have an opinion on whether or not we should change the route names. But if it seems like the new names are better, then we can do so without providing a BC layer. For XB, beta only means data stability, not API stability.
Question about the tags added in #5: what contrib project is this blocking?
Removing the "stable blocker" tag, because we have 📌 ComponentSourceInterface::submitConfigurationForm and validateConfigurationForm are never called Active as a stable blocker, and that's the only part of this that seems like a must have before a 1.0.0 release. Implementing #2 from this issue's summary or a hybrid solution allowing some blocks to be #1 and others #2 seems like it could happen after a 1.0.0 release. Please re-tag if you disagree.
Untagging "stable blocker" because I don't see why we couldn't add client-side validation at any time, including after a 1.0.0 release, but please re-tag if you disagree. We already have 📌 ComponentSourceInterface::submitConfigurationForm and validateConfigurationForm are never called Active as a stable blocker for the server-side part.
Closed 🐛 The navigation prevention inside the preview doesn't always work for code components Active as a duplicate of this. That one was tagged as a stable blocker, so transferring that tag to here.
Closing this as a duplicate of 🐛 Drupal.behaviors.xbDisableLinks doesn't disable links added to the DOM later Active .
Although we're working on Personalization in parallel to trying to get a stable XB 1.0.0 released, the former is not a blocker to the latter. If Personalization itself isn't sufficiently stable by the time we're ready to release XB 1.0.0, we can put it behind a feature flag.
Although we're working on Personalization in parallel to trying to get a stable XB 1.0.0 released, the former is not a blocker to the latter. If Personalization itself isn't sufficiently stable by the time we're ready to release XB 1.0.0, we can put it behind a feature flag.
Although we're working on Personalization in parallel to trying to get a stable XB 1.0.0 released, the former is not a blocker to the latter. If Personalization itself isn't sufficiently stable by the time we're ready to release XB 1.0.0, we can put it behind a feature flag.
Although we're working on Personalization in parallel to trying to get a stable XB 1.0.0 released, the former is not a blocker to the latter. If Personalization itself isn't sufficiently stable by the time we're ready to release XB 1.0.0, we can put it behind a feature flag.
Downgrading priority and removing the "stable blocker" tag because I don't think there's too many cases left where uncaught exceptions make it through to the generic "unexpected error" message. We can re-raise priority or re-tag as a stable blocker if we start seeing more of those. Regardless of priority or whether it's stable blocking, addressing this would be good for maintainability, so leaving that tag in place.
For XB 1.0.0, we don't need to support editing individual nodes, or other entity types other than Page, within XB. That's separate from being able to make content templates, so I commented on that separation in #3518272-3: [PP-1] Validate that content templates are attached to content entity types (+ bundles) that fulfill XB's requirements → .
We can add the ability to edit individual nodes, custom blocks, paragraphs, etc. within XB after 1.0.0. Until then, those things can be edited outside of XB.
Also, what might make sense here is for XB 1.0.0 to only allow content templates for nodes, and punt any support for creating templates for other entity types (e.g., paragraphs, custom blocks, etc.) to after 1.0.0.
The assumption the way this issue is worded is that XB should only allow content templates for entity types and bundles for which XB will be able to allow editing individual entities of. For example, only entity types that implement EntityPublishedInterface
. But I wonder if that's correct, so assigning to @lauriii. Because another way we can go here is to allow content templates for all entity types and bundles, even ones for which editing the individual entities can only be done outside of XB. For example, we could release XB 1.0.0 with the ability to edit content templates for nodes but not yet the ability to edit the individual nodes within XB. And if we can do that for nodes, then why not for content entity types that XB might never provide the ability to edit the individual entities of?
griffynh → credited effulgentsia → .
It's good for maintainability for our test coverage to include the most useful scenarios, but I don't think this needs to block a stable release.
This seems like a good issue for improving code quality/maintainability, but I don't see why it needs to block a stable release.
Just now realized we already had the "maintainability" tag, so perhaps re-using that is better than starting a new "code quality" tag.
Starting a new tag for issues that would improve overall code quality, but that I don't think need to block a stable release.
I think the original scope of this issue is done. #3535038: Auto-select or publish global Asset Library when needed to publish Code Components → is a child issue that would be a nice UX improvement, but I don't think we need to keep this meta issue open for just that. This issue's summary lists things for post 1.0 scope, but I think that should be a new meta issue and by definition not stable blocking.
Setting this to RTBC first before marking it Fixed in case there's something I'm missing and someone wants to make the case for why it's valuable to keep this open.
Removing the "backport" tag since it doesn't apply to what's in the issue title and #7 already cherry picked the part that's not in the issue title.
This is one we'll want to cherry pick to 0.x after it lands in 1.x, so tagging for that.
I love the creative thinking in #33, but some questions/thoughts:
The idea was to build a single component tree with the SDC and block components alongside the JS components and then use the preact render call on that inside the top level xb-preview custom element. Its a bit like what we're doing with hyperscriptify but on the preview side.
Are you thinking we only do this when we're previewing inside of XB? Or when viewing the page on the published site outside of XB as well? If the former, I'm concerned about the potential for the preview to lose accuracy if it's following a significantly different rendering path vs. viewing the page on the published site. If the latter, I'm concerned that having the whole page (including SDCs and Blocks) running inside of a single Preact VDOM tree ala hyperscriptify isn't sufficiently battle tested (us dogfooding it within XB UI for the right sidebar does count for something, but I don't think that gives us enough confidence in it to force it on people outside of XB UI) and might not be desired by people wanting to adopt JS components gradually. The advantage of Astro is that it's a mature and widely adopted framework where a lot of the potential bugs with nesting and intermixing interactive islands and static HTML have already been worked out.
The reason this is forcing us to chose between unwrapped and not is because we're treating each component as its own island
I don't see it this way, or rather, I don't see it as a negative. The way I see it, Astro is a well-loved framework for building content-driven sites, exactly the kind of sites XB is for. XB "just" adds the bonus of a really good UI and really good integration with a really good CMS. When building in Astro, you choose which components are server rendered (by default, all of them) and which are rendered as client islands (via a client:*
prop/attribute/directive). The former get rendered without any wrapper markup. The latter get rendered with the <astro-island>
and <astro-slot>
wrappers. Given how successful and well maintained Astro is, I don't really see any of that as a problem that we need to improve upon.
The unique thing introduced by XB is that within XB, people aren't writing .astro
files by hand, so we don't have client:*
directives to tell us which component instances need to remain wrapped. But we don't need to re-invent that from scratch either: React already defines the idiomatic way to do that for React components: the 'use client';
directive.
I think the earlier MR here (before I derailed us with comment #9) was on the right track. However, I think the logic needs to be: if the component has the 'use client';
directive, or if one of its dependencies does. But that needs to include dependencies that we're already tracking (i.e., other code components) as well as import dependencies that we're not yet tracking (swr
and next-image-standalone
). So perhaps the front-end could add some logic to add 'use client';
to a code component that imports either of those two? So the backend only needs to check for 'use client';
on the code component or its code component dependencies?
the unwrapping code is doing insertBefore rather than moveBefore which means we're losing events etc. moveBefore is only supported in chromium based browsers so we can't use that.
Yes, I can see why my original code that's in the issue summary is naive, since elements get inserted to the DOM, then removed, then reinserted. What a shame that moveBefore() isn't available on FF and Safari. But a new realization I just got is that ultimately what we're trying to emulate is SSR. So, what if we make <unwrapped-astro-island>
emulate that better, by using renderToString()
(same as server.js
within @astrojs/preact
) and then replacing its .outerHTML
with that? This would result in the rendered HTML getting inserted into the DOM just once.
#31 is probably contrived in that why wouldn't the component author just move the <li>
tags from ListItem
into List
, which would then be more robust to the item slots getting populated with anything, but there are probably real use cases out there that are less contrived, so #31 is just intended as illustrative.
A specific (though contrived) example... it should be possible to make a List
Code Component that does:
return(
<ul>
{item1}
{item2}
</ul>
)
And for item1
and item2
to be slots that get populated with ListItem
components that do:
return(
<li>FOO</li>
)
And so long as these are non-interactive components, for the DOM result to be:
<ul>
<li>FOO</li>
<li>FOO</li>
</ul>
Instead of:
<ul>
<astro-slot><astro-island><li>FOO</li></astro-island></astro-slot>
<astro-slot><astro-island><li>FOO</li></astro-island></astro-slot>
</ul>
Sorry, didn't mean to change the issue attributes.
I'd love to see a proof of concept of #26. I'm curious how you manage to get around the need for some kind of wrapper element for each component. React's createRoot()
and createPortal()
both require a DOM element container that they can completely take over. Does your idea somehow work without any such wrapper element? If so, even if it's bound to Preact and Astro internals, can you elaborate on the approach? Maybe we can find a way to implement a similar approach with the other Astro-supported JS frameworks as well?
I think the problem with #28 is that display: contents
doesn't affect CSS nth-child selectors and similar. So the presence of <astro-island>
elements isn't completely transparent. It's basically the <div>
soup problem, just with <astro-island>
instead of <div>
. Ideally, XB wouldn't insert any extraneous markup at all into the DOM, so that component authors are in 100% control of their markup.
I think it's okay for client-interactive components to have <astro-island>
wrappers: Astro is a popular framework, so this is easy to explain and point people to docs and blog posts about it. But for non-interactive components, Astro would normally SSR (or SSG) those, and ideally so will we once we add optional SSR support to XB, so the goal of this issue is to output a DOM that's the same as what we would end up having with SSR.
I haven't reviewed the whole MR, since others have, but the inputBehaviors.tsx
change looks correct, so I approved it since that was the only file not covered by other code owner approvals.
I would definitely not have expected useSWR() to require using 'use client'.
Same. But according to https://swr.vercel.app/docs/with-nextjs, useSWR()
is intended as a client API and is not supported by server components. And when I google search "does useSWR require 'use client' directive", Google's AI response is "Yes, useSWR requires the 'use client' directive in your component file". So us requiring it wouldn't break existing React ecosystem expectations with respect to this.
Given how close this MR is, tagging it as a beta target, because doing it after beta1 does involve a small BC break of site CSS, even if rare, per #3.
Other than #9, I think this MR looks great.
"Needs work" for the feedback above, but also a more philosophical question related to it:
If someone uses useState()
, then they need to explicitly add 'use client';
. That makes sense and I think is a standard requirement for any React codebase that uses RSC. However, are we okay that component authors will also need to add 'use client';
if their component calls useSWR()
even if it has no other client-side code? In principle we could eventually add support for useSWR()
to be SSR'able. But until we do, is it reasonable for component authors to have to know that our current implementation isn't and that's why they need to add 'use client';
?
I considered whether we should make #unwrap
check for either 'use client';
or useSWR()
, but the downside with that would be that if in the future we do make useSWR()
SSR'able and then change #unwrap
back to just be based on 'use client';
only, then that would be a BC break for existing components, since components that we were previously keeping wrapped with <astro-island>
would start getting unwrapped, so existing site CSS could break for these. Therefore, I think keeping #unwrap
tied to only 'use client';
is preferable in that component authors will be in control of if/when to remove their own 'use client';
directives on a component by component basis, once we add the ability for useSWR()
to be compatible with SSR'd components.
I guess my questions are:
- Does my reasoning above make sense?
- Even if it does, are we concerned about it being a WTF for component authors in the meantime needing to add
'use client';
for components that aren't interactive but need to fetch data?
I think everything I had in mind when I wrote #11 landed already, so setting this to RTBC, but please re-postpone it if there are other tedious-to-rebase MRs we're still wanting to get in before this one.
penyaskito → credited effulgentsia → .
We're not targeting this for beta anymore. I'd still like to resolve it relatively soon after beta1, but the "stable blocker" tag plus Critical priority is enough to keep this on our radar for that.
We're not targeting this for beta anymore. I'd still like to resolve it relatively soon after beta1, but the "stable blocker" tag plus Major priority is enough to keep this on our radar for that.
"Needs work" for the question about refetch
above.
I would personally prefer the Drupal wide consistency (no_ui) than the SDC wide consistency (noUi). But this is not a strong opinion.
I would have agreed with this if we didn't already have thirdPartySettings
in SDC yaml but third_party_settings
in config. But given that we have that, I think it would be weird to make no_ui
the outlier for a key that we don't camelCase.
Note to reviewers: please review this as normal, but once this gets to RTBC, instead of merging it, please set it to Postponed. There's a few MRs in the queue that we'll want to merge in ahead of this one rather than requiring tedious merging/rebasing of them.
wim leers → credited effulgentsia → .
I suspect what's happening here is that we determine all the component and slot dimensions on page load and in a resize observer, but I wonder if CSS animations interfere with that. Adding @jessebaker to this issue because he wrote most of the current code that we have for this.
Probably worth opening a core issue to propose adding the no_ui
key to SDC definitions. We don't need to hold this issue up on that actually landing in core, but it would be good to get some initial feedback on it from SDC system maintainers.
Something along the lines of `hide_from_ui: true` property in the SDC schema
Field types and Views plugins can have a no_ui
key in their plugin definition, so we can continue with that pattern here, just in the SDC YAML instead of in a PHP attribute.
@lauriii and I discussed that getting the image component DX right should actually block the beta, so I tagged 📌 Create an Image SDC that can be included by other SDCs Active as such and promoting this one as well.
@lauriii and I discussed this and decided to make it a beta blocker. Every design system includes components with images, so providing the XB recommended way to do that is important to include in the beta.
There's a comment in that Mozilla issue that mentions https://github.com/guybedford/es-module-shims, a polyfill for multiple imports (and other import map features) for browsers that don't support it natively.
"Needs work" for test failures, but if you're wanting additional reviews ahead of fixing tests, please feel free to set back to NR.
Code looks great, but the test failure seems likely to be legit.
effulgentsia → made their first commit to this issue’s fork.
The adding of the internal: prefix to user-entered paths that start with / normally happens in
LinkWidget::massageFormValues()
. Are we skipping that step for component instance forms for widgets that have client-side transforms?
I'm not positive, but I think the answer to this is yes, because GeneratedFieldExplicitInputUxComponentSourceBase::clientModelToInput()
assumes that client-side transforms are responsible for replicating that kind of server-side widget processing. Based on this assumption, here's a quick and dirty patch for what I think would solve this, but I haven't tested it. Marking "needs review" for feedback on approach, even though this is just an untested patch and not a proper MR.
wim leers → credited effulgentsia → .
The cause of this is that the value of the inputs
column for a link prop is stored as: {"uri":"/page/1","options":[]}
, but it should be stored as {"uri":"internal:/page/1","options":[]}
. Outbound processors are only run on paths in the internal:
scheme. The adding of the internal:
prefix to user-entered paths that start with /
normally happens in LinkWidget::massageFormValues()
. Are we skipping that step for component instance forms for widgets that have client-side transforms?
Per #13.
The ADR isn't beta blocking, so untagging.
Thanks. Tagging as a stable blocker because this is needed by Drupal CMS.
if someone changes the site homepage and also something else in system.site (site name?) how can we reflect that easily in one auto-save
I don't understand what the concern is. Can you elaborate?
I like the multiple actions support in this MR, but I'm confused why we're limiting to a single target. I imagined actions
following the same format as in a recipe, where its direct children are targets and then children of each target is actions on that target. Is there a compelling reason to deviate from how the actions
key works in a recipe?
Related to above, do we need to couple the target and ID? Couldn't the ID be anything? For example, for the use case of XB UI setting the homepage, could it be something like xb_set_homepage
without being coupled to system.site
?
I didn't test this MR, but the code looks great to me! I'd consider it RTBC once #22 is addressed.
@larowlan: Re #33, can you think of any other steps to reproduce what was fixed in this issue (a different boolean prop of a non-boolean field)? I think @mayur-sose tested with Pathauto because that's what's still in this issue's summary.
The Navigation component added here isn't the only way to trigger the bug in 🐛 Drupal.behaviors.xbDisableLinks doesn't disable links added to the DOM later Active , but it's now a common way, so adding that as a related issue.
effulgentsia → created an issue.
I closed 📌 [PP-2] Create UI for selective discarding of changes Active as a duplicate of this per #15.
This is already incorporated into the MR in 📌 Create UI for selective publishing of changes Active .
No, we'd want to strip ?alternateWidths=…
We can strip this from what the Twig renders into the src
attribute, but we don't have to. I think it's okay for the Twig to just do src="{{ image.src }}"
as the MR already does and for that to mean the query parameter is retained. The query parameter is harmless other than adding some extra bytes to the HTML output, and those extra bytes are minor compared to what we end up needing in srcset
anyway.
This MR still needs, within ShapeMatchingHooks.php
, where we set the expression for srcSetCandidateTemplate
to instead change the expression of src
to be such that it evaluates to a URL that includes the alternateWidths
query parameter. Eventually stuff like this could be done with AdaptedPropSource
but I don't know if we have adapters working sufficiently well yet, so it might make sense to instead add another computed property to ImageItemOverride
. XB's shape matching and prop expressions are kind of a tricky part of XB, so perhaps @wim leers could help out with this part?
I think @larowlan would have some insight into #13. Seems like our handling of "use this example value that's already the prop shape but isn't a Media reference" for images should also work for videos, but apparently something about it isn't.
wim leers → credited effulgentsia → .
FYI: I opened 📌 Build Breadcrumb and SiteBranding code components Needs work as a child issue of this one, and re-parented 📌 Build a navigation code component Active to be a child issue of this one as well.
I opened 📌 Build Breadcrumb and SiteBranding code components Needs work as a sibling issue.
I tagged this as beta blocking because 🌱 Plan how to evolve code component overrides Active is, and this is part of that.
effulgentsia → created an issue.
"FE implementation" is ambiguous, so re-titling this issue to clarify its scope. ✨ Add a Video prop type to the Code Component editor Active is a separate FE issue for the video prop that's a completely separate scope.
See also ✨ Add a Video prop type to the Code Component editor Active
effulgentsia → created an issue. See original summary → .
I opened 📌 Change video prop shape: remove width and height, and make src optional Active as a follow-up based on discussion with @lauriii and @balintbrews.
effulgentsia → created an issue.
Overall I think this MR is really good. My one question/concern is: are we sure we want to limit it to just being able to run a single action? I think allowing it to be a sequence of config actions would be potentially more useful to us in the future, even if the current use-case is just a single action. To make it a sequence of config actions would require decoupling the ID from the action. Which I think would be okay, because load() doesn't need to instantiate an entity that doesn't already exist, does it? Couldn't we instead make load() load from auto-save without making any assumptions about the nature of the ID?
If we want to punt on ^ to a follow-up, in order to not hold up this MR, I'd be okay with that.
There's an MR here but it's failing tests, and lots of commits happened on 0.x late last week, so would be good to bring those in (via either merge or rebase).
📌 EntityFormController should load auto save state if it exists Active is in. Is this issue now fixed as well, or is there more to do here that the other one didn't cover?