- Issue created by @ultrabob
- Open on Drupal.org βCore: 9.5.x + Environment: PHP 8.0 & MySQL 5.7last update
about 1 year ago Waiting for branch to pass - @ultrabob opened merge request.
- Open on Drupal.org βCore: 9.5.x + Environment: PHP 8.0 & MySQL 5.7last update
about 1 year ago Waiting for branch to pass - Status changed to Needs review
about 1 year ago 12:29am 13 September 2023 - π―π΅Japan ultrabob Japan
I'd love a review on this. Probably the simplest way to test it would be to:
- Use drush to generate a module
ddev drush gen module
- Use drush to add a custom entity type to the new module, you can accept all the defaults except you need to make sure the entity is revisionable
ddev drush gen entity:content
- install the module
- add a workflow to the entity type
- manage display for the new entity type and confirm that workflow buttons is an option there, drag it into the list of visible elements
- Create one of the new entities, view it, and confirm that the workflow buttons appear
The module currently tries to load a "default" form for the entity, but since that is not often in the set of forms configured with a content entity, I've set it to fall back to the edit form. It may be better to just always use the edit form.
- Use drush to generate a module
- Status changed to Needs work
about 1 year ago 9:17pm 13 September 2023 - π―π΅Japan ultrabob Japan
As we've begun using this further, I find that the route filtering isn't enough in workflow_buttons_form_alter to remove the unnecessary fields everywhere they shouldn't appear. Views that show entities will end up showing the whole form instead of just the workflow buttons. I'm working on finding a better solution.
- Open on Drupal.org βCore: 9.5.x + Environment: PHP 8.0 & MySQL 5.7last update
about 1 year ago Waiting for branch to pass - Status changed to Needs review
about 1 year ago 11:06pm 13 September 2023 - πΊπΈUnited States mlncn Minneapolis, MN, USA
Nice! I have some questions out of ignorance:
Is every entity going to have an edit form, or should we having an additional try statement here in case they don't?
Come to think of it, should we be trying to load the edit form first and falling back to default? (As i note you suggested in #3)
Any idea if any of this works with Layout Builder?
Also it seems our pseudofield gets added to every view mode display which we really don't want, i wonder if we can do an update hook to remove this so people have to add it when and where they want it.
Thanks for all this! Will try to be testing more thoroughly shortly.
- πΊπΈUnited States mlncn Minneapolis, MN, USA
Hmm it should already be defaulting to not displaying but the site i'm checking things out on predates that fix; i think never mind that comment.
- Open on Drupal.org βCore: 9.5.x + Environment: PHP 8.0 & MySQL 5.7last update
about 1 year ago Waiting for branch to pass -
mlncn β
committed f57c2af8 on 8.x-1.x authored by
ultrabob β
Issue #3386885: Show buttons on Custom Entity view pages (replacement...
-
mlncn β
committed f57c2af8 on 8.x-1.x authored by
ultrabob β
- π―π΅Japan ultrabob Japan
Thanks @mlncn. I don't think it works with Layout Builder, because Layout Builder removes the display settings entirely. I suppose what would need to happen for layout builder support would be to create a block with the buttons in it, that can be added to the layout. I can imagine a case where an edit form doesn't exist. I imagine it would be pretty rare, but I'm not sure what we could do if it didn't exist. The current behavior is to throw an InvalidPluginDefinitionException. I'm not sure what we could do with another try-catch block there except provide a clearer exception. That is probably a good idea.
I note that you committed the code, would it make sense to also mark the issue fixed? I see both this issue and the original one adding the functionality for nodes appear to be outstanding though they are committed. Is that something you clean up when you cut a release?
- πΊπΈUnited States mlncn Minneapolis, MN, USA
Yeah i tested it and confirmed an improvement so far. Would like to test more and clean up some things before cutting a release.
Layout Builder creates blocks for pseudofields but we need to test that our way of adding or not is consistent, and i am concerned about still having to load the edit form on content view since we don't have an easy way to see what is actually wanted within layout builder.
Right now though i'm mostly distracted by suddenly no longer seeing a good use case for simply displaying the buttons as-is when not on the edit form. "Publish" still makes sense but variations of "Save" don't. I know i had a use case at one time but i need a refresher from my past self, or someone else!
@ultrabob Are you interested in co-maintainership by any chance?
- π―π΅Japan ultrabob Japan
@mlncn Sorry for the long delay in replying. I've been out sick and then recovering from that. I've needed this on two projects now, so let me lay out my use case.
We are building some kind of approval flow, for which some of the approvers don't need to edit anything, they just need to indicate whether it is good to move forward or not. We send notification mails (and also list pending items on a dashboard) to those users when the content is ready for their review, and want them to see the content in a view mode, not on an edit form. After reviewing it, clicking a button to move the content on to the next state is all we need from them. On occasion we may need to alter the action to pop up a modal and ask for some more detail, but for most transitions, clicking the button and not changing the content is all we want them to be able/encouraged to do.
Thank you for the offer of co-maintainership. I'm afraid I don't have much bandwidth for active maintainership right now. I was learning a lot about this area of Drupal while submitting the MR request too, so I'm far from an expert. Let me set up notifications for issues on this module, try my hand at helping out on the issue queue for a while, and if it seems like something I can meaningfully contribute to, I'll inquire with you about whether you'd still like another maintainer?
By the way, I also ran into the issue with display being on by default, probably because that is not in the latest beta. if it would be better for me to change that line in this MR too, I'm glad to do it/see it done.
- π―π΅Japan ultrabob Japan
@mlncn I can't promise a ton of active maintainership time, but that is true for most of us. If you are still interested in my help, I'd be glad to help out.
- Status changed to RTBC
12 months ago 4:03am 13 December 2023 - π―π΅Japan ultrabob Japan
Since it has been merged and other fixes built on it, I think it is good to go at least RTBC.
- Status changed to Fixed
8 months ago 11:44pm 25 March 2024 Automatically closed - issue fixed for 2 weeks with no activity.