Thanks @heatherwoz I have marked it in my calendar and am planning to be there. I've confirmed I have the necessary YouTube upload credentials in place. I'm looking forward to it!
Thanks @mark_fullmer I looked at the commit and confirmed there is nothing there to be alarmed about. Back to RTBC
I installed and enabled the gin theme on a site with workflow buttons enabled, enabled sticky action buttons, and confirmed the issue. Applying this patch solved the issue, the code makes sense, and will not break non-gin sites, so I'd say this is ready to go in!
Thank you I was actually going to suggest the same thing. There is one more open issue about removing some variables that were being made available to the theme but not used anywhere, so we could remove those at the same time. I always have the best of intentions to get in and help move these issues forward, but as this issue makes obvious, I'm not very successful with it.
I also wondered whether it would be a better approach to provide a formatter widget for the content moderation display rather than providing a new one, but I couldn't figure out in the time I had remaining how to make that work. I do think that providing options on the widgets that content moderation provides may be a better approach for both edit and display side modes. I can't really think of a scenario where a person would want both if we fixed the capability of showing the current moderation state.
One thing that could be possible here, might be to offer config for transitions to exclude from workflow buttons in the display mode. That would require some thought as to how to present that in an intuitive way.
@joseminosa, I'm sorry for the very slow reply. Are you reporting that the other issue is important, or are you reporting a new issue besides the one in that other issue?
I don’t recommend adding an “Edit” button or replacing the “Draft” button with an “Edit” button. If a user is allowed to edit the content, they probably already see an “Edit” button in the usual place.
Usually, workflow buttons are used for things like approvals. In these cases, the person approving the content should see the final version, not the edit form. These users often cannot edit the content at all — and even if they can, editing is not the action they are meant to take with the workflow buttons.
That said, your situation might be different. If you really need to add the “Edit” button there, you can do it using a form alter.
I should mention, this is a breaking change. The form id changes, so any form alters that were relying on the form _id would cease to work. This is likely slightly lessened by the fact that, at least in my experience, the most common alter is to remove a button for saving into the same state. Still I have projects where I override button labels for some users, and this change would break them. I’m not sure there is an update hook that could remedy that.
Thanks for the report, I see what you mean, and assume you are talking about the default workflow when in a published state. A draft button doesn’t necessarily mean a button that doesn’t do anything, meaningful though I agree it normally would be. Transitions can be named whatever you want. If we were to consider this change we’d want to consider removing buttons that save into an unpublished state that also does not become the default revision. That change might make sense in many cases, but I can imagine flows where it wouldn’t. Say a request to unpublish button, which then moves the content to an editor with final say over what gets unpublished. I think your likely best bet for this would be to do a form alter to remove the offending button and add an edit button if that matches your need.
I finally had time to work on this some more. I think it is ready for review.
This patch adds a dedicated form implementation to handle workflow transitions on entity view pages, addressing several longstanding issues with the current approach.
Changes
New DisplayForm Class
- Added a new DisplayForm class that implements a form specifically for displaying
workflow buttons on entity view pages
- Properly manages transitions, filtering out self-transitions (e.g., draft to draft)
to reduce confusion
- Includes comprehensive unit tests for the new form
Enhanced Entity View Display
- Modified workflow_buttons_entity_view() to use the new DisplayForm instead of reusing
the entity form
- Added pre-check to verify transitions are available before rendering the form
- Significantly simplified the form rendering logic
Improved Code Quality
- Added type declarations and proper docblocks throughout
Unit Tests
- Added comprehensive unit tests that verify:
- Form construction with and without available transitions
- Self-transition filtering
- Form element structure and attributes
- Entity revision creation and state transition handling
- Form submission workflow
Benefits
- More focused implementation that only shows relevant workflow buttons
- Cleaner separation of concerns between entity forms and workflow transition forms
- Improved reliability in displaying workflow buttons on entity view pages
Related Issues
This addresses problems with:
- Workflow buttons sometimes displaying redundant options
- Workflow transition operations not being properly handled
- UI issues with the display of workflow buttons on entity view pages
I've added Gitlab-CI to the Merge Request and cleaned up coding standards issues. This should be fully ready to go. If you want to add me as a maintainer I'd be glad to cut a new release. Otherwise, consider this a +1 on RTBC
ultrabob → made their first commit to this issue’s fork.
To be sure that it wasn't a core version I updated my local to 10.4.3 and can still reproduce the error. I have the same field attached to three content types and the field placed as a block in layout builder. I have attached the field storage config, and the config for the field on the landing page content type which is the content type I'm trying to view when I get an error. I only get the error if the page I'm trying to view has content in the field. I've also attached the block config, just to try and make sure you can see what I'm doing here.
Thanks. I'm on Drupal 10.4.1 locally. The field is not in a paragraph or anything like that. It is in a field group on the form.
I want to get this answer to you quickly, but after that I'll dig in and try to identify exactly which field on what content type is giving me those errors so I can provide more detail.
@apmspooner thanks for your prompt attention to this issue. Very much appreciated. Since you also bumped the module version on the MR, I had to do an upgrade of the module before I could test. I can't promise that issues I report are due to the original issue and not the new version of the module, though it seems that they are.
I first confirmed the issue you pointed out with caching. With my patch in place, If I updated the title of a referenced node, the title was not updated in the custom field display.
After upgrading the module, applying the new patch, running database updates, and clearing cache. Viewing a page with the field placed in block layout (note, not layout builder, I'm placing the field as a block). I get the following WSOD error:
The website encountered an unexpected error. Try again later.
LogicException: Render context is empty, because render() was called outside of a renderRoot() or renderInIsolation() call. Use renderInIsolation()/renderRoot() or #lazy_builder/#pre_render instead. in Drupal\Core\Render\Renderer->doRender() (line 303 of core/lib/Drupal/Core/Render/Renderer.php).
Drupal\Core\Render\Renderer->render(Array) (Line: 481)
Drupal\custom_field\Plugin\Field\FieldFormatter\BaseFormatter->getFormattedValues(Object, 'en') (Line: 35)
Drupal\custom_field\Plugin\Field\FieldFormatter\CustomFormatter->viewValue(Object, 'en') (Line: 353)
Drupal\custom_field\Plugin\Field\FieldFormatter\BaseFormatter->viewElements(Object, 'en') (Line: 91)
Drupal\Core\Field\FormatterBase->view(Object, 'en') (Line: 268)
Drupal\Core\Entity\Entity\EntityViewDisplay->buildMultiple(Array) (Line: 282)
Drupal\layout_builder\Entity\LayoutBuilderEntityViewDisplay->buildMultiple(Array) (Line: 226)
Drupal\Core\Entity\Entity\EntityViewDisplay->build(Object) (Line: 461)
Drupal\Core\Entity\EntityViewBuilder->viewField(Object, Array) (Line: 243)
Drupal\Core\Field\FieldItemList->view(Array) (Line: 174)
Drupal\ctools_block\Plugin\Block\EntityField->blockAccess(Object) (Line: 127)
Drupal\Core\Block\BlockBase->access(Object, 1) (Line: 124)
Drupal\block\BlockAccessControlHandler->checkAccess(Object, 'view', Object) (Line: 109)
Drupal\Core\Entity\EntityAccessControlHandler->access(Object, 'view', Object, 1) (Line: 329)
Drupal\Core\Entity\EntityBase->access('view', NULL, 1) (Line: 63)
Drupal\block\BlockRepository->getVisibleBlocksPerRegion(Array) (Line: 138)
Drupal\block\Plugin\DisplayVariant\BlockPageVariant->build() (Line: 270)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 128)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
call_user_func(Array, Object, 'kernel.view', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.view') (Line: 186)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 116)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 90)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 741)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
The formatting here was confusing, but I've tracked it back to line 481 of src/Plugin/Field/FieldFormatter/BaseFormatter.php
$rendered_value = $this->renderer->render($value);
Changing that line to be
$rendered_value = $this->renderer->renderRoot($value);
appears to get rid of the error, and also in some brief testing seems not to exhibit the caching issue you pointed out.
I had missed the alternate suggestion of renderRoot in the original error. I just now read up on the difference, and renderRoot seems the more appropriate choice here.
There are more formatters using this pattern, but I'm unsure if this change is needed with them. Apologies for not having more time to dig in.
I confirmed renderInIsolation with renderRoot, and confirmed that that works as well. So I've updated the merge request. I have not reviewed all the formatters to see if others need to be updated. I've only updated the items that we ran into under time pressure.
We ran into this same issue again with a URL field type, so I've added a commit to apply the same fix to the URL formatter.
ultrabob → created an issue.
ultrabob → created an issue.
ultrabob → created an issue.
ultrabob → made their first commit to this issue’s fork.
Thanks Mariam and GGH!
ultrabob → made their first commit to this issue’s fork.
Remove an errant PHP version number. I have not done anything to address the tons of content at the bottom of the page that indicates that it needs an update.
hestenet → credited ultrabob → .
hestenet → credited ultrabob → .
hestenet → credited ultrabob → .
hestenet → credited ultrabob → .
hestenet → credited ultrabob → .
hestenet → credited ultrabob → .
That all makes sense to me. I'm also not an expert on all the caching and context details here. I just know that when I tried to place the field as a block in block layout, I started getting the DWSOD. Given that this was happening with an Entity Reference field, I wonder if the core Entity Reference Field Formatter has an example we can learn from. Maybe the solution is to detect whether there is a render context, and only renderInIsolation or renderPlain when it isn't present. I appreciate you being responsive on this. It is a very cool module, that I became familiar with due to a talk at PNW Drupal Summit and then again a bit later at BADCamp last year.
I realized after submitting this that there is a new 3.1 release, so I tried installing that to see if it fixed my issue. It did not, but the patch applies to the new version and still works, but now I get a bunch of Warnings such as
Warning: Undefined array key "wrappers" in Drupal\custom_field\Plugin\Field\FieldFormatter\BaseFormatter->getFormattedValues() (line 477 of modules/contrib/custom_field/src/Plugin/Field/FieldFormatter/BaseFormatter.php).
and
Warning: Trying to access array offset on value of type null in template_preprocess_custom_field_item() (line 119 of modules/contrib/custom_field/custom_field.module).
so I've reverted to 3.0.x
I've created a merge request with the suggested change in place.
ultrabob → created an issue.
That fixes it. I have a few more improvements I want to make in the code, and then I'll put out a release for this. Thanks!
Thanks for that follow-up. We may need to consider a different selector for the gin theme. Any further detail or a patch would be greatly appreciated.
I'm sorry to hear that you are having trouble. I just tested with /node_*/ in the box, and the preview button was gone for all content type add and edit forms. Are you including the quotes? (The quotes should not be there)
kthull → credited ultrabob → .
I'd be glad to take this on again, assuming all works out to be there. Since I have the recording kit, some of it would be more straightforward next year.
For me, locking the module to 1.10 and adding this to my composer.json seems to get it to ignore the problematic patch.
"patches-ignore": {
"goalgorilla/open_social": {
"drupal/views_ef_fieldset": {
"Issue #3173822: Exposed operators are not included in fieldsets": "https://git.drupalcode.org/project/views_ef_fieldset/-/merge_requests/1/diffs.patch"
}
}
},This has broken our deploys for open_social as well, and I'm not sure how we could go about fixing it.
I'm not sure what the latest compatible version of views_ef_fieldset is. I've seen an issue saying that 8.1.10 is incompatible, so it is hard to construct a replacement patch, but even then, I'm not sure if we can patch the open_social composer.json.
As helmo points out, Gitlab patches should never be linked because they tend to disappear like this one did.
liberatr → credited ultrabob → .
I ran into this issue on an Open Social site, where a flexible group creation form had a taxonomy term entity reference field. Without the patch, there was no $form[$key]['#parent'] which led to a WSOD. The patch here showed the error message in the wrong place instead of the entire form white screening.
Confirming that patch #6 fixes the issue, but do not have time at the moment to contribute a test case to support the fix.
liberatr → credited ultrabob → .
I tested this with Drupal 11, and ran into an error when using the widget in the display due to the assert asserting the wrong type. I've fixed that issue, and tested that the module now works as expected with Drupal 11.
ultrabob → made their first commit to this issue’s fork.
Thank you for pointing that out. It has been addressed in the 2.0.x branch.
In that case, how about:
Node Menu Placer provides a new UI to facilitate the quick placement of nodes in complex menu structures. This module works with the Menu Link Weight module to update the menu settings of the node form.
ultrabob → created an issue.
I'm not sure why the RectorBot created 5 Drupal 11 tickets, but I found a minor bug, fixed the tests, and marked the module as Drupal 11 compatible. I'll do a release shortly.
As long as the functionality of this module, doesn't interfere with how ckeditor5's autogrow functionality works, I'd say we go ahead and update it given that the only change that seems to be needed is updating the version in the info file.
ultrabob → made their first commit to this issue’s fork.
@robloach, I have confirmed that this issue exists, but this change doesn't seem to fix it. In my testing editMenuMenuParent was always defined, but an error happens later around line 193 where we try to get the parent menu title:
let currentParentTitle = currentOption.innerText.substring(
currentOption.innerText.indexOf(' '),
);currentOption has just been defined as null. so getting innerText is not possible. I think this is a pretty big problem of a bug, but I don't have the time to try and debug it at the moment.
ultrabob → made their first commit to this issue’s fork.
hestenet → credited ultrabob → .
hestenet → credited ultrabob → .
hestenet → credited ultrabob → .
hestenet → credited ultrabob → .
hestenet → credited ultrabob → .
hestenet → credited ultrabob → .
hestenet → credited ultrabob → .
hestenet → credited ultrabob → .
The status of how to do it via UI is still unclear, and the github issue has been closed, saying that there are details in a JIRA ticket. Here is how I accomplished the translation for french using the config files:
Create a new file called core.entity_view_display.private_message.private_message.inbox.yml in your default config/language/<langcode_here> directory. In my case, the contents of this file are:
langcode: fr
content:
created:
settings:
past_format: 'il y a @interval'Make sure to update the langcode value if you are translating a different language.
Create a new file called core.entity_view_display.private_message.private_message.default.yml in the same directory. Similarly, my contents were:
content:
created:
settings:
past_format: 'il y a @interval'
Import config, and the private messages interface should be translated properly. I did not translate the future_format because I never expect to receive a private message from the future. You can translate that by adding the future_format key just above the past_format key.
I came here looking for deprecation details. It is essential to provide guidance about how to determine whether the module is safe to uninstall.
This has been merged, so I'm marking it as fixed.
ultrabob → created an issue.
ultrabob → made their first commit to this issue’s fork.
Info out of the starshot initiative webinar today: This is not possible. We'd need to provide a separate recipe to do it, recipes cannot be shipped as part of modules.
ultrabob → made their first commit to this issue’s fork.
Workflow Buttons now passes all linting and code analysis with no warnings or errors. There is another issue for adding tests, given that the existing tests were not testing Workflow Buttons features.
ultrabob → created an issue.
The drush recipe generator has never actually worked for me. Is it just me, or should we remove that link?
In the issue description, it says that "site feature" is commonly used, but the wordpress.com example is using "your site's features." These are just English words used together in a sentence, not a unit that is commonly used. I'm not sure what the benefit of prepending "site" to "feature" is, unless this issue is a proposal for core or Drupal CMS recipes only, which is not clear from the description. Feature seems a better alternative than site feature.
I'm likely missing some context that isn't in the description, but it seems to me, that just like modules and themes, recipes will need short descriptions for both their page on drupal.org, and eventually for project browser. I'm not sure that mandating a name is all that useful or enforceable.
I think it is hard for the name to provide enough information about the type of things done in the recipe, the intention of the recipe, and the scope of the recipe. I feel like it would be better to focus on better descriptions, and consider some type of mechanism for automating a summary of the actions a recipe will take in a way that it can be more easily absorbed than by reading the yml file, etc.
Something along the lines of headings like:
Modules Installed
Config Modified
Actions Invoked
Content Added
with summaries in them. We wouldn't want to dig into deep detail, but could give a summary that should along with a short description, give a good idea that the recipe does what you think it does.
In the issue description, it says that "site feature" is commonly used, but the wordpress.com example is using "your site's features." These are just English words used together in a sentence, not a unit that is commonly used. I'm not sure what the benefit of prepending "site" to "feature" is, unless this issue is a proposal for core or Drupal CMS recipes only, which is not clear from the description. Feature seems a better alternative than site feature.
I'm likely missing some context that isn't in the description, but it seems to me, that just like modules and themes, recipes will need short descriptions for both their page on drupal.org, and eventually for project browser. I'm not sure that mandating a name is all that useful or enforceable.
I think it is hard for the name to provide enough information about the type of things done in the recipe, the intention of the recipe, and the scope of the recipe. I feel like it would be better to focus on better descriptions, and consider some type of mechanism for automating a summary of the actions a recipe will take in a way that it can be more easily absorbed than by reading the yml file, etc.
Something along the lines of headings like:
Modules Installed
Config Modified
Actions Invoked
Content Added
with summaries in them. We wouldn't want to dig into deep detail, but could give a summary that should along with a short description, give a good idea that the recipe does what you think it does.
tekNorah → credited ultrabob → .
@wylbur thanks a lot for your work on this. It looks like at this point we only have three minor phpstan warnings left, which we could add to the baseline and try to fix later, but given that they don't look like hard issues, I think I want to fix them so we can start with fully passing gitlab-ci.
Thanks for the review wylbur! I started trying to fix the tests, and when I looked into them deeper, it appeared to me that the tests had been copied from elsewhere, and were not testing the functionality of this module. We need a follow-up issue to add some test coverage for the module.
ultrabob → created an issue.
I fixed some cspell issues and some of the test issues, but ran out of time to solve the menu_link dependency fail. It probably needs a composer.json added so that menu_link will be available to the tests to get the unit tests to progress further
ultrabob → made their first commit to this issue’s fork.
ultrabob → made their first commit to this issue’s fork.
The BOF schedule description for the coffee exchange says "be sure to register below" which may be leftover from something else, or maybe I'm missing a link somewhere? It may not be possible to change, but I thought I'd point it out, and this seems like the best venue for that.
This is related to the Testing Techniques page linked in Related Pages. It confused me to click on the Testing Techniques page and end up in the DeGov distribution documentation. I recommend that we either make it clearer, that that is where the link is going or come up with a version of that page for Drupal core.
It confused me to click on the Testing Techniques page and end up in the DeGov distribution documentation. I recommend that we either make it clearer, that that is where the link is going or come up with a version of that page for Drupal core.