This is what has been running for a few months for the picky client, and they are very happy with it! I'd like to start using it in more places too. I do find it the ideal compromise between say media_entity_download where you have full control over the path but Drupal needs to be in the loop every time and media_entity_file_replace
Missed that question but i will participate in https://www.drupal.org/project/issues/site_moderators → as Gisle mentioned.
Merge request !13162 is a straightforward reroll— exactly what is in daniel korte's patch initial patch in #2 🐛 Adding attributes to views-view-list.html.twig doesn't work if Views List class Style option is empty Needs work got into core through changes made out of this issue (without tests?!), so technically the problem stated in the title of this issue is now fixed in Drupal core. My re-roll of a merge request therefore only preserves jwilson's proposal in #12 🐛 Adding attributes to views-view-list.html.twig doesn't work if Views List class Style option is empty Needs work , implemented in gauravvvv's patch in #24 🐛 Adding attributes to views-view-list.html.twig doesn't work if Views List class Style option is empty Needs work , to not blow away attributes that were added by a possibly earlier preprocess.
I agree this is how it should work, but am not up for a test, let alone making this consistent everywhere, and i know there are rumblings about somehow getting rid of hook_preprocess
entirely, however that would work, so i leave this humble re-roll and update as my offering.
Opened a followup for the SpamSpan change (which, as noted, is not in config, and i don't think needs to be) instead of Invisimail: 📌 Switch to using SpamSpan if available instead of invisimail Active
Brought into both branches.
Thanks, still looks like the best fix for the moment.
Yep all it needs.
yep got it
Ah so that is what the problem is. Wherever that is documented i must always get that wrong, despite having tried in the past both with manual instructions and the rebase button in GitLab; it had seemed the catch-up rebase itself was the problem. Thanks. Also thought i had fixed it in https://git.drupalcode.org/project/redirect/-/merge_requests/164.diff (in that it was one line and useable, at least).
Either way, despite the maddening fork PLUS branch setup of Drupal issues, yes straightforward re-roll (the lines above had been changed to be better self-documentation) and back to RTBC.
mlncn → changed the visibility of the branch 3342409-redirectformnodeformalter-calls-getinternalpath to hidden.
mlncn → changed the visibility of the branch 3342409-avoid-internal-path-call-on-unrouted-url to active.
mlncn → changed the visibility of the branch 3342409-avoid-internal-path-call-on-unrouted-url to hidden.
mlncn → created an issue.
Based on 🐛 Fix accessibility issues by using visually-hidden on hide and unsetting title element on remove Needs review that is the functionality the module has now— `visually-hidden` on Hide, and fully removed for Removed. So this is all set?
Frustratingly, instead of being committed, this now needs a re-roll :-/
Very nice improvement. Went a baby step further and changed "reorganize" to "reorder" for flat vocabularies.
Thanks for the work and the review!
Made some copy-edits to the form (including describing the newly added feature from [ 📌 Hide "Add child" link from entity operations Active ] and put the use statement in order.
Would welcome follow-up efforts to convert this to the new (11.2+ only) way by using a new OOP hook for the help text.
Thank you!
While no Drupal.org-hosted module needs a composer.json (and the dev branch can now be installed via composer because we have switched it to a supported branch name (2.0.x) allowing us to make an official dev release → ), i agree that modules should have a composer.json primarily because it makes uising issue forks of modules much easier.
mlncn → created an issue.
Looks good; sorry for the delay everybody.
The commands can be seen and run. The main need seemed to be the drush.services.yml
, which is not supposed to be needed since Drush 13, but there is documentation and then there is what actually works. I'm still getting errors ("> [warning] Undefined array key "success" batch.inc:229; [error] Error: Call to a member function claimItem() on null in _drush_batch_worker() (line 242 of /var/www/html/vendor/drush/drush/includes/batch.inc) #0 /var/www/html/vendor/drush/drush/includes/batch.inc(204): _drush_batch_worker()"), but that might be with my plugin not the new Drush implementation.
Ah, Drush 13 changed things up: "Drush 13 expects commandfiles to use Autowire to inject Drupal and Drush dependencies. Prior versions used a drush.services.yml file which is now deprecated and will be removed in Drush 13."
Also getting what seems to be errors due to this with content moderation and paragraphs. (Running the drush command for Cached moderation state → module brings up a bunch of "An existing default revision of the 'paragraph' entity type can not be changed to a non-default revision." errors. Seems there should be a change record with some hints of what to do with content created before this was enforced?
I've only just started using it myself, but maybe Batch Plugin → module would make it easier to do this work through the UI, with Drush, or on cron, which is i guess what i need, to initialize cron queue batches from an install hook.
Welll Features may never export namespaces correctly? But six years on we may as well do it correctly ourselves.
This is an accessibility issue, that the descriptive label (say, "Date of birth") is not connected to the form label immediately connected to the form field for a screen reader (will only say "Date").
OK this is 100% cosmetic changes to the code formatting.
Seems the best solution, despite adding one more nested div, is to add the form-element template as a wrapper, which should let all themes treat form items as they thing they should be treated.
Happy to add a configuration option for this, disabled by default, if at most we want an optional change that does not impact existing sites at all.
mlncn → created an issue.
Confirming the code quality and functionality of this patch. Also think that it is the right solution, and a good one. In particular for a vocabulary where a user does not have edit permissions for the terms, the inapplicable operation link to "Add child" is very in-the-face and distracting.
Back to needs review at the very least, for the patch in #14 as per #20
It was RTBCd (comments #15 and perhaps #18) and it still works 7 years later! To address #16 the module could offer an interface for capturing keycodes but clearly that is not needed to address the immediate pain of Coffee stealing a common browser shortcut. And there is no need to prompt administrators to configure the keys because there is an update hook to assign all the currently available keys by default.
Improvements that could be made in follow-ups but should be considered out of scope for this patch which allows site administrators to making Coffee more compatible with browsers and more usable for more people:
- Allowing any key code to be used.
- Providing a helper for capturing keycodes.
- Allowing individual users to set their own keycodes.
Changing the issue title to make clearer that last one in particular was never the intent of any of the patches contributed here.
Thank you so much! It definitely was not healthy there for a minute and very much needed this report!
Thanks! Are you using the module actively in Drupal 11 or simply helping out here? Thanks!
Tested, fixed follow-up error, and tagged a release, 8.x-1.3.
Thank you for the report, so sorry for releasing a Drupal 11 version that was not actually compatible.
Gentle reminder that rather than proclaiming a module "dead" this is a golden opportunity to contribute a fix → . Although, on second thought, that is thousands of words making clear how complicated it is to contribute what should be the simplest of things. Ah well. Fix coming shortly.
mlncn → changed the visibility of the branch 2906496-give-media-a to hidden.
This problem of the Drupal\pathauto\PathautoItem object (contrib, i know, but it extends Drupal\path\Plugin\Field\FieldType\PathItem with minimal changes not relevant here) having an alias property of NULL is also an issue for validation even when automatic aliases are enabled.
The updated workaround/fix is now in a brand new branch with a merge request that has only what it should have.
Still do not know how the alias property can end up NULL in the first place – when the values array has the alias correctly in the very same object – so not sure how to add a test, but the fact that all existing tests pass indicates that it is a harmless failsafe.
mlncn → changed the visibility of the branch 3535695-url-alias-field to hidden.
mlncn → created an issue.
mlncn → created an issue.
mlncn → created an issue.
Thanks ultrabob! Yeah, the Trash module is really the only contender out there now i think (and it is in Starshot / Drupal CMS) but it still has some edge cases with … content moderation … that have some sites avoiding it right now: ✨ Trash is not compatible with Content Moderation Active
Probably Trash will become 'the' solution and maybe even get into Drupal core, but building on Workflows to introduce a 'deleted' (trash) state still seems more core-compatible and future-safe right now.
We'll remove it from 2.0.x and put the change in the release notes.
It is of course unlikely to get these variables into core quickly so we should create a contrib module, but should also file a core issue in the Workflows queue like beunerd suggested.
To belatedly provide the use case for this variable, in a node--full.html.twig
:
{% if not node.isPublished() %}
{% if current_revision_state == 'trash' %}
{% set version_state_message_title %}
{% trans %}
This {{ content_type }} has been deleted.
{% endtrans %}
{% endset %}
{% set version_state_message_subtitle %}
{% trans %}
You can restore it by editing.
{% endtrans %}
{% endset %}
{% elseif node.isDefaultRevision() %}
{% set version_state_message_title %}
{% trans %}
This {{ content_type }} is not yet viewable by the public.
{% endtrans %}
{% endset %}
{% set version_state_message_subtitle %}
{% trans %}
Please complete and publish it soon!
{% endtrans %}
{% endset %}
{% else %}
{% set version_state_message_subtitle %}
{% trans %}
This {{ content_type }} {{ current_revision_state }} has not yet been published.
{% endtrans %}
{% endset %}
{% set version_state_message_subtitle %}
{% trans %}
An older version is published; please complete and publish this latest {{ current_revision_state }} soon!
{% endtrans %}
{% endset %}
{% endif %}
<section class="hero is-danger is-bold">
<div class="hero-body">
<div class="container">
<p class="title">{{ version_state_message_title }}</p>
<p class="subtitle">{{ version_state_message_subtitle }}</p>
</div>
</div>
</section>
{% endif %}
Applied to both (in progress, somewhat disruptive) 2.0.x and 8.x-1.x (we should tag a point release there soon) branches.
Fix looks correct. Thanks!
mlncn → created an issue.
This latest, greatest, and i hope final update to the merge request shows the label (when it exists, which in my testing is always) and what bundle it came from, and if there are more bundles.
field_example "Example" on node:page & 1 other
Also, it sorts by field machine name within entity types, but leaves fields grouped by entity type. We now show the entity type with the bundle (media:image, node:article, user:user - in the same manner displayed in the Bundles field below) so it should become evident quickly that all the media fields are first, then node fields, then user, for example.
For me, definitely the most confusing thing was not knowing what entity type each field name was on, since fields are not shared across different types of entities, so the exact same name can appear twice. Showing the entity type as well as the bundle definitely helps but i still think grouping by entity type is clearer.
As a follow-up we may want to add help text that the label may vary on the additional bundles (where we note "& 1 other" or "& 2 others" etc for each field), but putting "label may vary on Y other bundles" made the already-long option text too long.
The other follow-up that could improve UX is reminding people they can filter for which bundles will have CER active for below. (So field_media_image "Image" on node:page & 5 others can be synchronized with media:image for node:article only.) Also, that saving the form after selecting the fields will update the autocomplete options in Bundles to only the relevant ones (i don't think it works the other way around, but a way to filter the first and second fields by bundle would be nice).
Back to RTBC since it was only me with the problem for quite an edge case and the fix is tiny and safe.
All right, by default user
in core is an entity type without bundles, definition getKey('bundle') returns an empty string. That will always work if an entity does support bundles so we can simply not add the bundle restriction on an empty string.
(Alternatively, we could change the UI to not offer "user:user" etc for entity types that do not support bundles; but i think the fix here in the sync is better and more straightforward.)
Update to the merge request in a minute.
Retroactive updates for past, already saved, previously existing content is a huge feature. Took me a while to find this issue and i was surprised the question of what happened with past (when the form field for A to B is empty only because it had not been synced when A was updated because CER was not configured yet, will it clear B to A when B is saved?)
Unfortunately in testing this merge request with the 5.x branch of the module, for a node to user entity reference with corresponding user to node entity reference, i get this error:
Drupal\Core\Entity\Query\QueryException: '' not found in Drupal\Core\Entity\Query\Sql\Tables->ensureEntityTable() (line 373 of core/lib/Drupal/Core/Entity/Query/Sql/Tables.php).
Drupal\Core\Entity\Query\Sql\Tables->addField() (Line: 58)
Drupal\Core\Entity\Query\Sql\Condition->compile() (Line: 177)
Drupal\Core\Entity\Query\Sql\Query->compile() (Line: 82)
Drupal\Core\Entity\Query\Sql\Query->execute() (Line: 145)
Drupal\cer\Form\CorrespondingReferenceSyncForm->getEntityIdsToSync() (Line: 72)
Drupal\cer\Form\CorrespondingReferenceSyncForm->submitForm()
call_user_func_array() (Line: 105)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers() (Line: 43)
Drupal\Core\Form\FormSubmitter->doSubmitForm() (Line: 589)
Drupal\Core\Form\FormBuilder->processForm() (Line: 321)
Drupal\Core\Form\FormBuilder->buildForm() (Line: 73)
Drupal\Core\Controller\FormController->getContentResult()
call_user_func_array() (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->{closure:Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber::wrapControllerExecutionInRenderContext():121}() (Line: 622)
Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->{closure:Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber::onController():96}() (Line: 183)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 116)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 90)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 53)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 715)
Drupal\Core\DrupalKernel->handle() (Line: 19)
Essentially the fix by wojtha in this issue 13 years ago and richthegeek and cvangysel in #873070: When an image button appears after another button in a form, the wrong triggering element and #submit handlers are detected → 14 years ago remains correct and needed.
When image buttons are clicked, browsers do not set NAME in the POST, instead setting NAME_x and NAME_y (with x and y the coordinates of the click). To get the clicked button we need to test for these parameters instead of the presence of value— that is, equally . (Damien's suggestion of using #value
for image buttons also does not work, whether the button is pressed or not, whether it has a value or not, the value gets set anyway, and set to "true", presumably because of how image buttons are treated by the browser— the only thing that is safe to check are if coordinates were set.)
And yes (replying to 13 year old comments from another issue now, but the same feedback will come up) these changes make the alternative condition specific to image buttons, but i'm fairly certain that is because that is all there is— the only other element in core which uses #has_garbage_value
is Radios, which aren't ever going to be triggering a form submission.
So, yeah, image buttons are ridiculous and Drupal core could totally deprecate them and not support them since basically nobody has cared for a decade (especially if we finally do 📌 Use form element type instead of Needs work ), but as long as we have ImageButton extending Submit we need to support it properly as an alternative to Submit, that works alongside Submit buttons.
The merge request is ready and works, but leaving Needs work as it needs a test showing that this fails even when value is not set in the initial form element definition. As my use case is altering an existing button there is some possibility that something about that is adding 'value' back, but definitely unsetting value is not enough.
The merge request fixes the Gin display issue reported.
Note image buttons do not work the same as regular submit buttons in core. Right now, the image button always steals the submit handler! So maybe this cannot work, so although the display and the functionality are distinct issues that can be handled independently leaving this as active rather than needs review until there is a core fix / workaround (to address core functionality re-opened ✨ UX when using gin with Contnet Moderation and Workflows Active ).
And it is not a duplicate with changing values client side; although that may be related, this is being caused with alteration in a Drupal form/widget alter.
This is absolutely still a bug in Drupal 11.
Button Field module documents:
Important note
Due to #1452894: Elements with #has_garbage_value and #value are always set as a triggering element you can not use image buttons on edit forms when there are any other buttons. Non-editable displays are unaffected.
And where i'm coming from, i confidently told Gin in 🐛 Image buttons cannot be kept in Gin's sticky form actions Active that core's image button form element is a valid alternative to a standard submit input form element:
As noted in ✨ UX when using gin with Contnet Moderation and Workflows Active , Workflow Buttons module pairs very well with Gin for improving the experience of using content moderation workflows, and the 2.0.x branch of Workflow Buttons' "Trash workflow" submodule switches to using the image button to have a trash icon for the soft-delete, similar to the trash icon Claro/Gin add to core's delete link. […]
But in short swapping the submit button out and substituting an image button for it works, and Gin should support this core form element when adding Gin attributes and looking for the
#gin_action_item
flag.
And i get it working perfectly and then discover that no matter which button i press, the image button (soft delete!) is the one that gets incorrectly identified as the trigger button.
Thanks Arun!
Did you enable Admin Toolbar Extra Tools? drush -y en admin_toolbar_tools
Back to needs review then, this looks like. Great work, thanks!
Updating Drupal 11 status of modules in table, start to make other version headlines clearer.
mlncn → created an issue.
If you don't scroll back further in the code for the root error, TypeError: $(...).fitVids is not a function
is probably the error you will see in your console for this problem.
The patch fixes FitVids.
mlncn → created an issue.
Thanks— i had followed the issue fork commands to make https://git.drupalcode.org/issue/drupal-3535695/-/tree/3535695-url-alias...
But when i went to submit the merge request, it said 178 commits, 0 changes, so that means i did something wrong and need to try again, yes?
This fixes it but no i have no idea why it would be broken in the first place.
Noticed when making this patch i do still have a core patch in the vicinity: "Path module calls getInternalPath without checking if the url is routed #3342398": "https://git.drupalcode.org/project/drupal/-/merge_requests/9209.diff", from 🐛 Path module calls getInternalPath without checking if the url is routed Active but it does not look like it intersects.
Likewise looked at #2484411: Manual path aliases are not the same as aliases on the node form → but could not see a connection.
Meant to mention the site has no multilingual or translation set up at all (at least not currently, it is possible it did at one point long ago and they were disabled) so 🐛 If you don't want to translate your URL alias, the original URL alias won't work with your translations Needs work should not be the issue.
Also meant to mention explicitly that this does not pose a problem as long as automatic aliases are turned on; our internal tracking issue once we realized the problem was not, say, a contact form no longer existing, only losing its alias, went from being titled "Issue with URL Alias" to "Content that had a manual (rather than automatic) URL alias has no alias shown in the form when edited, so on save the alias is lost" with the deathless description "Manual URL Alias are being forgotten.".
mlncn → created an issue.
Thought i should report
🐛
Remove last facet fails
Active
as related to here— TLDR i think the "prettiest" paths (no query string at all, /view/category/foo-258
in the example above) seem a little incompatible with Facets via Better Exposed Filters or Views itself.
This issue was filed first, but the same solution is already RTBCd in 🐛 Function maxlength_field_widget_third_party_settings_form() should return array, not null Active
These issues are duplicates but whichever is marked as fixed should give credit to the contributor(s) of the other.