tim.plunkett → made their first commit to this issue’s fork.
Added https://git.drupalcode.org/project/drupal_cms/-/commit/f2ca0f5566e3739de... after the fact
okay, so more like 144 hours ;)
Adding @mandclu!
tim.plunkett → made their first commit to this issue’s fork.
As a former/inactive maintainer, I'd be happy to add @mandclu as a maintainer.
Based on your response @dakala, sounds like you agree as well.
I will wait 48 hours and then add @mandclu unless @dakala says "go" or "no" sooner :)
Updated to indicated assigned status
Assigning to track lead (co-lead with @_doyle_, picked first one alphabetically)
Assigning to Pam representing the Leadership Team
Assigning to track lead.
Assigning to track lead.
Assigning to track lead.
Assigning to track lead.
Assigning to track lead.
Assigning to track lead.
Assigning to track lead.
Assigning to track lead.
Assigning to track lead.
Kristen Pol → credited tim.plunkett → .
Great work, all!
Reviewed
quietone → credited tim.plunkett → .
Great work @phenaproxima!
tim.plunkett → made their first commit to this issue’s fork.
chrisfromredfin → credited tim.plunkett → .
tim.plunkett → made their first commit to this issue’s fork.
Merged !511 🎉
Wim Leers → credited tim.plunkett → .
Wim Leers → credited tim.plunkett → .
@phenaproxima and I discussed this over the past few days, this is ready now.
larowlan → credited tim.plunkett → .
Adding credit
tim.plunkett → created an issue.
Thanks!
tim.plunkett → made their first commit to this issue’s fork.
from @longwave in Slack:
MRs against 11.x are ok and will be preferred once 11.0.0-beta1 is out and 11.1-only changes go into 11.x (yes this is confusing, can't wait until we can use
main)
tim.plunkett → created an issue.
tim.plunkett → made their first commit to this issue’s fork.
@catch that would explain it!
@alexpott I would have thought the `->reveal()` calls would have helped, but fair enough
I'm not able to run the tests-only job for some reason. But when I do locally, I get
Failed asserting that Double\ConfigurableInterface\P1 Object (...) is not an instance of interface "Drupal\Component\Plugin\ConfigurableInterface".
Which sounds like a Prophecy mixup, not an actual working assertion.
@alexpott am I overthinking it? The change looks good
tim.plunkett → created an issue.
tim.plunkett → created an issue.
I will try to look between now and the end of DrupalCon. For now, I'd consider this NW for finding a better way
lauriii → credited tim.plunkett → .
`needs design` literally means that it needs a designer. Which is different than usability.
I don't know that a UI designer ever looked at this, as opposed to solely front-end and back-end developers.
ckrina → credited tim.plunkett → .
Say you carefully configure your view display to have the exact order and field formatters you want. You save, everything is great. Then you wonder what might happen if you turn LB on. You try it, decide against it, and then disable it.
In HEAD, all is well. you're back to your perfectly configured display.
After this MR, everything is gone. You're back to square one.
Left some suggestions for nits, but also raised a few points that need to be addressed.
What's the stack trace of the exception? Whatever it is, it's happening before layout_builder_entity_type_alter() has run, which shouldn't be possible.
I don't remember which hooks we had or if they were "standardized" yet, but 6 years ago when I opened this, I said:
Allow the derivers themselves to affect the list of cache tags used.
This will remove the need for external code (like a hook) to clear the cache.
So I don't know that this has any practical application anymore if we're okay with needing hooks forever
Looks good, thanks
amateescu → credited tim.plunkett → .
I've asked @omkar.podey to mark resolved threads with ✅ so @srishtiiee can close them
Test
Merged! Thanks
Saving credit
tim.plunkett → changed the visibility of the branch 2293803-replace-confirm-password to hidden.
Answered the question, fixed the nit, and performed my own review. Thanks @kunal.sachdev!
tim.plunkett → made their first commit to this issue’s fork.
@pyaephyohein that's 🐛 PHP notice "Undefined index: region" on layout overrides when using the Field Layout module Needs work , different module.
Thanks @clayfreeman and @godotislate for keeping this one going.
Running the 'tests-only' pipeline shows a fail as expected. Keeping NW for the IS update
+1 to RTBC, the Drupal.ajax vs ajax discussion in the MR can be resolved
@effulgentsia and I discussed this just now.
We're fine with committing this, but think that this needs it's own CR pointing out that if you choose to use closures in your config targets, then you will lose the ability to have any sufficiently complex AJAX handling on that form (anything that approaches the complexity of a multi-step form).
This is mitigatable from two directions:
- Simplify your config forms to not be multi-step
- Rewrite your closure as a static method
I don't know of any forms that would need the advice of #1.
And taking the example of SiteInformationForm, there is no reason that fromConfig: fn($value) => $value ?: ini_get('sendmail_from'), couldn't be a static method on the form...
Consider this a +1 from the Form/AJAX system maintainers, and if you want to relive some of the digging I did on this, enjoy reading all the child issues of #635552: [meta issue] Major Form API/Field API problems →
+1 for fixing the "Displays Settings" typo. The new layout is great!
As this is targeting 10.3 and won't be backported to 10.2, here's a patch for 10.2
tim.plunkett → created an issue.
+++ b/token.tokens.inc
@@ -569,7 +569,7 @@ function token_tokens($type, array $tokens, array $data, array $options, Bubblea
+ if (($url_tokens = \Drupal::token()->findWithPrefix($tokens, 'url')) && $node->id() !== NULL) {
Could also use !empty($node->in_preview). That pre-dates LB (think original node preview) but is also set when LB generates the sample entity.
#284 that is being worked on in the one child issue: 📌 Restrict access to empty top level administration pages for overview controller Fixed
Removing resolved "needs" tags
"Bumping" is unhelpful, now the issue is further down the RTBC queue
#12-14, this is a meta/plan issue, please open a dedicated issue to propose that change.
The changes in !5094 seem reasonable, but are unrelated to this issue. Nothing in that changeset has anything to do with modals. I'd ask that you create a new issue for your refactoring so we can focus on one MR to accomplish implementing modals.
@alexpott pushed back on explicit test coverage for double-saving, but the last commit fixes things nicely enough
One piece of feedback. Also there are 17 other unresolved threads that *were* resolved. @godotislate as the MR author, you can mark those as resolved.
smustgrave → credited tim.plunkett → .
Attempted a rebase now that combined forms is in. I did not fix the bugs introduced in the last commit
tim.plunkett → made their first commit to this issue’s fork.
Reviewed all the changes since the end of September, and I'm +1 to this being RTBC.
I especially like the Subform changes, I think this marks the first core usage outside of plugins.
Thanks to everyone who worked on this, but especially @srishtiiee who carried it for so long, and to @lauriii and @alexpott for getting it to the end (I hope it's the end!)