tim.plunkett → created an issue.
If a committer could fix the spacing on the @todo before merge, that'd be cool. But this looks great, thanks!
Looks great, thanks for the follow-ups!
No meeting happened that week, due to no agenda. No credit for me please!
This needs steps to reproduce, or it can be closed.
My expectation is that if you never interact with the Sort option, every tab should be sorted by their default sort.
But if you select a specific Sort for one tab, it should persist across tabs when possible.
For example, if you select Z-A on one tab, it should be used across all tabs.
Looking into this
Reviewed, merging.
tim.plunkett → made their first commit to this issue’s fork.
alexpott → credited tim.plunkett → .
Removing tags from the dupe
This has a legit merge conflict after 🐛 Selected categories disappear on switching tabs Active went in, not just on the compiled svelte.
catch → credited tim.plunkett → .
Re-targeted!
gábor hojtsy → credited tim.plunkett → .
xjm → credited tim.plunkett → .
xjm → credited tim.plunkett → .
@baluertl, I see how you mistook @dww's comment as him "losing his temper", but he was stating a fact. Feature branch commit messages can be anything, and that's okay.
The point I am not clear on is why this wouldn't affect contrib.
Contrib projects have the same commit message format as core, and have for as long as I can remember.
I would expect contrib to adopt this change too.
Merged!
tim.plunkett → made their first commit to this issue’s fork.
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!