NW for the comment
looks good to me.
we do the same with the main ckeditor script for similar reasons
there is a merge conflict to resolve
Given that this was a regression and the actual implementation had to change based on CKE evolution, this needs a proper test to make sure the fix works after a CKE update
Can you show the comment form with the texarea and the submit button before/after, not only the posted comment ? thanks!
Committed and pushed 1630acb886 to 11.x and 660f3bd724 to 11.1.x and 1f81a9dd69 to 10.5.x. Thanks!
I thought we wanted to upgrade twig in 10.5.x, if that's not the case, no need for this backport
Committed and pushed 554b7c600ad to 11.x and b5ccec7567d to 11.1.x and 07ae29edb49 to 10.5.x. Thanks!
needs reroll
Committed and pushed 6265e217854 to 11.x and 94d0636ceaf to 11.1.x. Thanks!
+1 for doing that in gin
pameeela → credited nod_ → .
pameeela → credited nod_ → .
the test can be removed here. it's testing that something we removed was removed
saving credits
Thanks for sorting out the credits!
left a question
Which branch should be reviewed?
how does the public comment form look after this change (under an article for example)?
small nit left, it's a bit longer but it's more consistent.
It used to be that an id must start with a letter. I see that the restriction is now lifted, short term you can pass the selector through CSS.escape() as in
const id = 'a98f0826-a717-4967-8b1c-8528dcd4113';
once('foo', `#${CSS.escape(id)}`);
Sorry I didn't see the new comment before committing. that's what I get for keeping my tabs opened too long.
lint issue
I'm seeing several issues related to using Olivero as an admin theme. it was never meant to be used that way.
@djsagar could you explain the use-case a bit more? I'd rather not have people spend time on making Olivero work as an admin theme, and instead work on Claro or Gin
Committed and pushed 903f73fb30 to 11.x and fbad2d37f6 to 11.1.x and 431532d69a to 10.5.x and 8c1d5599ef to 10.4.x. Thanks!
small change and it's good to go
Thanks for moving this forward @tame4tex, i checked the tests they make sense so +1 for them. Webform overrides some of the states API methods so any patch here will need an update on webform side
I replicated 🐛 Using a computed twig as a value for a visibility condition breaks in Drupal 10.3 Active and made a test locally and the MR fixes the issue. all good here.
For the JS I think it's solid, elements and event handlers are cleaned up so memory shouldn't leak. I wish we could make the code simpler but I don't have a better way (:
The scope is a bit too narrow, I think we can add to the theme/SDC maintainers.
we need @mogtofu33 to confirm reading the Drupal core governance, and agreeing to the responsibilities listed under:
and a MR :)
So in 11.x and yarn 2 we don't check dependencies because the command doesn't exists. From the docs:
NOTE: The command yarn check has been historically buggy and undermaintained and, as such, has been deprecated and will be removed in Yarn 2.0. You should use yarn install --check-files instead.
The --check-files option doesn't exist anymore.
the deduping doesn't impact our vendored deps so I'd be inclined to just ignore it and remove the check. i'll try to fix it later today but might just remove that
automation issue is at 🌱 Set up a formal process for ensuring JavaScript dependencies remain up to date Needs review
I don't see what to change in Drupal core, hence setting this as a support request.
The easiest way to go about this is exclude Drupal.t from being replaced. Using a library with attribute type: module shouldn't aggregate them so you won't get aggregation or minification for theses
Shouldn't that be called mergeResponse?
Porting credit
Committed and pushed e5a5bfe0d5 to 10.5.x and bd6031cb2f to 10.4.x. Thanks!
This will also allow themes to be much more reusable, making it possible to deploy a SDC-based theme configuration through a recipe
Added a link that explains a bit more of the framework https://fs.blog/playing-to-win-how-strategy-really-works/
Looks good to me
@nicxvan eventually we'll need to discuss specifics, but for now the focus is really on the overall strategy. First do you agree with the goal? it might not look like it but it's a pretty big change to become a CMS builder (and not "just" a site builder). Did we miss anything in the "where will drupal core play" section? any worries about the realignment of Drupal Core with regard to Drupal CMS, with the CMS market, or even with the broader open-source and commercial world. Does the "how will we win" section make use of all the strengths of Drupal in general, are we missing something to achieve the goal? I think it's more this kind of things we're looking for at this point.
The discussion about the specific APIs, render arrays, theme and hooks is to low level at this time. It'll be easier to discuss once we have a frame of reference we agree on to make decisions. If it's burning your fingers to talk about it right away, a new issue would be best.
Committer team gave some feedback in private that would need to be addressed before we can un-postpone
nice, didn't know we had that
Committed 3ef3cfe and pushed to 11.1.x. Thanks!
Cherry pick is not clean on 10.5.x, need a MR for it
Committed and pushed f99f198961 to 11.x and a67ab83394 to 11.1.x and 0aeedd16e5 to 10.5.x and 73487835e2 to 10.4.x. Thanks!
it's one string that needs translating, it'll handle all inline fields. No visible changes to users after this patch
The translation approach has been validated in #22, so we won't go with the CSS method. The string translation workflow is well oiled and adding a different way to do this doesn't provide significant benefits.
Here we need to add a context to the string, I suggest: "Field label"
we can add markup for this
smustgrave → credited nod_ → .
Committed and pushed 1c33460a8c to 11.x and f1139711e4 to 11.1.x. Thanks!
hang on, yarn vendor-update was not run on 11.x, lots of changes to the cke sources
we can get the max input var value and check with JS at submit for example. It's a pretty dangerous situation so special casing makes sense
hook_requirement make sense. i would prevent the form from being submitted with a warning if we know it will go over
nice thanks
The main issue is at the http/php level
@tedfordgif we have done all we can at the html level. no more cruft to remove. the shadow inputs are not submitted
yes makes sense to backport to 10.4
we can fix the uis where this makes sense, like the cell for tabledrag, the translation table, etc. not a by defa,ult thing
you're right, reverted
probably a false positive
The mr should target the 11.x branch
very nice, I like that a lot.
the list needs to be categorized (the query objects should probably be transformed to views in our case) and prioritized (based on the usage across the different wordpress block themes
this makes the bot crash, i don't have 10G or ram on it
we would need more info from the logs, watchdog or server logs to know what's going on.
Committed and pushed 94bc96150c to 11.x and 59e2130adf to 11.1.x and c5f9eea85c to 10.5.x and 3e4bcd62df to 10.4.x. Thanks!
Adding a bit more details to the format