Committed and pushed 6f6a78544ee to 11.x and ec3d2e1a81d to 11.1.x. Thanks!
+1 from me
all child issues are fixed, should this one be closed too?
sorry another conflict because of the inline comment length patch
merge conflict
committed 📌 Fix DrupalPractice.Objects.GlobalFunction in module Postponed first, conflict in phpcs
need a rebase unfortunately, I'll try to merge soon after it's back to a mergeable state
Committed 3ac8317 and pushed to 11.x. Thanks!
Thanks a lot for getting that resolved!
Another motivation was to prevent people from adding MB of js that we have to bubble around during rendering, so making it hard to add things was part of the goal of I remember correctly. Now it's less of an issue with lazy builders and all the other things we have available
Not sure when I gave feedback about this issue. looking at the difference between jQuery and our code, they have a shortcut when a single element is parsed, which we didn't replicate.
I like the template solution, we should try it out and see if the tests are green. We might need an additional test to make sure we don't break this again.
As for revert, given that this code has been shipped in several minor and major releases for over half a year, It's easier to fix it here and ship the fix in the next release. Nobody should have to rewrite their code because of this change, otherwise there'd be a change record attached to the issue.
Committed eeaa76e and pushed to 11.1.x. Thanks!
The cherry pick is not clean on 10.5.x
Committed fda19f8 and pushed to 11.x. Thanks!
Looks reasonable to me
first piece of code pushed
pameeela → credited nod_ → .
if the issue is out of the usual core issues lifecycle title should be updated per: https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... → t That'll also stop the needs review bot from posting on this issue
scope increase is fine but we don't have a test to ensure link order, might want a followup for that
Committed ba0fa0e and pushed to 11.x. Thanks!
nod_ → made their first commit to this issue’s fork.
CI is green but pre-commit checks are not, and can't commit. Getting
Running PHPStan on changed files.
------ -----------------------------------------------------------------------------------------------------------------
Line modules/filter/src/Plugin/Filter/FilterHtml.php
------ -----------------------------------------------------------------------------------------------------------------
268 Method
Masterminds\HTML5\Parser\Tokenizer@anonymous/modules/filter/src/Plugin/Filter/FilterHtml.php:265::setTextMode()
has no return type specified.
🪪 missingType.return
------ -----------------------------------------------------------------------------------------------------------------
[ERROR] Found 1 error
Can we make the follow ups before the commit? Thanks!
thanks @quietone and @pameeela!
is it possible to check the availability of nodejs on the shared hosting while looking at all this? just a yes/no would be great
shouldn't the values be merged? or are we sure the GET data is always more up to date?
thx for the update
After
Drupal\Tests\field_ui\FunctionalJavascript\DisplayModeBundle 2 passes 60s
Drupal\Tests\field_ui\FunctionalJavascript\ManageDisplayTest 3 passes 136s
Drupal\Tests\field_ui\FunctionalJavascript\ManageFieldsTest 7 passes 211s
Drupal\Tests\field_ui\FunctionalJavascript\EntityDisplayTest 3 passes 72s
Drupal\Tests\field_ui\FunctionalJavascript\DefaultValueWidge 1 passes 24s
Before
Drupal\Tests\field_ui\FunctionalJavascript\DisplayModeBundle 2 passes 32s
Drupal\Tests\field_ui\FunctionalJavascript\ManageDisplayTest 3 passes 136s
Drupal\Tests\field_ui\FunctionalJavascript\ManageFieldsTest 7 passes 211s
Drupal\Tests\field_ui\FunctionalJavascript\EntityDisplayTest 3 passes 72s
Drupal\Tests\field_ui\FunctionalJavascript\DefaultValueWidge 1 passes 29s
haven't checked all tests but it seems fine, except for DisplayModeBundle
ah yes sorry, was about to commit when i spotted it :)
- I like that it still works without JS
- fix on the JS needed
- Any idea of the perf hit on the tests? it feel like it'd be significant
- this CR is not up to date anymore (version): https://www.drupal.org/node/3400352 →
- can we reduce the description text from https://www.drupal.org/node/3503549 → , it's too long and makes it hard to see what changed
question in MR
merge conflict
merge conflict
Fixed a new failure on commit from 🐛 Navigation Top Bar should Edit button as the primary action when viewing a forward revision Active
Committed b450cb3 and pushed to 11.x. Thanks!
Should this meta add the phpcs rule, or we leave that to a child issue?
The other issues are in
Committed dcfacb2 and pushed to 11.x. Thanks!
thanks!
We're changing strings in tests that do not run? Seems like a cleanup is needed
The code itself looks good. It's nicer to use slots than mess with theme functions. I'm still on the fence adding shadow dom on a custom core element.
Given the fact that it's an experimental theme that we'll retire at some point, why not. I would like to have a disclaimer somewhere in the code (a twig comment above the <template> element maybe?) saying that this is a test, not a guideline on how to use template/shadow dom in Drupal. We haven't talked through all the topics (how/where do we document the slots, js required, style duplication, different styling methods, etc.) hence the disclaimer. Most of the topics would need to have an answer or general agreement before we use this in non experimental parts of core. in the spirit of getting things going I'm ok with committing this
NW for the disclaimer comment
Committed 74bdd70 and pushed to 11.x. Thanks!
we might want that for 11.1.x too, cherry pick isn't clean
thanks for the extra test, in this case as this is a purely UI concern we do not need to make sure the text is not present. we removed it and keeping it in a test makes the string stay in the code. Kinda defeat the purpose of removing it :)
Committed and pushed 8357990ca57 to 11.x and 74321e2f21f to 11.1.x. Thanks!