- Issue created by @jurgenhaas
- Merge request !479Issue #3463796 by jurgenhaas: Ensure sticky action buttons to work with modals... โ (Open) created by jurgenhaas
- Status changed to Needs review
about 1 month ago 2:05pm 25 July 2024 - Status changed to RTBC
about 1 month ago 7:01pm 29 July 2024 - ๐ฎ๐ณIndia Tirupati_Singh
@jurgenhaas, I've applied the MR !479 as a patch and it applied with no errors, After applying the patch, the action buttons are being rendered when
Enable sticky action buttons
theme setting option is enabled. I have attached the before and after fixes video clips for reference. Since the MR resolves the issue moving the issue status to RTBC. - ๐บ๐ธUnited States devkinetic
+1 working splendidly for me as well, thanks for this patch.
- ๐จ๐ญSwitzerland saschaeggi Zurich
saschaeggi โ changed the visibility of the branch 3463796-ensure-sticky-action to hidden.
- ๐จ๐ญSwitzerland saschaeggi Zurich
saschaeggi โ changed the visibility of the branch 3463796-ensure-sticky-action to active.
- Status changed to Needs work
about 1 month ago 8:07pm 1 August 2024 - ๐จ๐ญSwitzerland saschaeggi Zurich
Unfortunately after intensive testing I discovered some issues. The easiest way to reproduce this is when you edit a field in an entity type, e.g. https://gin.lndo.site/admin/structure/types/manage/article/fields/node.a...
and change the
Allowed number of values
This triggers an Ajax request and breaks the Save button. Additionally once the Ajax request gets executed the form actions get appended to the end of the form.
- ๐จ๐ญSwitzerland saschaeggi Zurich
Just a thought: As the disallow list seems to grow every day I think the best solution might actually be to use an allowlist and an opt-in system. This way we can control better which forms we're altering.
As I don't see a solid solution in Drupal core to check if an Ajax request happens in a modal/dialog/offcanvas.
- ๐ฉ๐ชGermany jurgenhaas Gottmadingen
I'll have a look later today. I would hope that we should be able to recognise if we are in a form rebuild, which should be handled differently, i.e. not handling any action button. I'll get back when I know more.
- ๐จ๐ญSwitzerland saschaeggi Zurich
I would hope that we should be able to recognise if we are in a form rebuild, which should be handled differently, i.e. not handling any action button.
Agreed, but the buttons get re-rendered in an Ajax call in Drupal, as Drupal alters the form and form elements ids with adding a unique hash and this also includes the form actions ๐
- ๐ฉ๐ชGermany jurgenhaas Gottmadingen
I've rebased the MR first, that's done.
Agreed, but the buttons get re-rendered in an Ajax call in Drupal, as Drupal alters the form and form elements ids with adding a unique hash and this also includes the form actions
That's correct, but we could probably handle that. I've just tried manually and that seems to work:
- Ignore action buttons completely in that context so that the updated form will be sent without the action buttons that received special handling originally. That keeps those in place where they had correctly been placed originally.
- Hook into the ajax response and add an update command to search and replace the
form
attribute in action buttons to replace the old form ID with the new one.
@saschaeggi do you have an idea where and how we could achieve step 1? I could then look into the second one.
- ๐ช๐ธSpain ady1503
Versiรณn de Drupal
10.3.2
Gin 8.x-3.0-rc13When I activate:
Enable sticky action buttons
Displays all actions of the form in the sticky header.There are no save or cancel or dellete buttons on the views popup configurations.
- ๐จ๐ญSwitzerland saschaeggi Zurich
This was fixed, check out the latest dev release.
- ๐จ๐ญSwitzerland saschaeggi Zurich
@jurgenhaas
@saschaeggi do you have an idea where and how we could achieve step 1? I could then look into the second one.
I don't think that's easily solvable.
Can you push your changes? We got some more issues in, and I lean towards switching to a opt-in mode.
- ๐ฉ๐ชGermany jurgenhaas Gottmadingen
I lean towards switching to a opt-in mode
While that would certainly be the easiest way forward, I'm concerned about both the DX and UX. Most developers won't opt-in and the result will be a terrible mixture for the end-user. The finish line seems to be so close, I'd like to give this another try. I'm a bit busy the next couple of days, but I'll give this another go right afterwards, if that's OK with you.
- ๐จ๐ญSwitzerland saschaeggi Zurich
While that would certainly be the easiest way forward, I'm concerned about both the DX and UX. Most developers won't opt-in and the result will be a terrible mixture for the end-user. The finish line seems to be so close, I'd like to give this another try. I'm a bit busy the next couple of days, but I'll give this another go right afterwards, if that's OK with you.
Completely agree here. Let's collaborate here. Just want to make sure we either find a solid solution or we may continue with opt-in and postpone all forms for after a stable release ๐ค๐
- Status changed to Needs review
10 days ago 1:26pm 29 August 2024 - ๐ฉ๐ชGermany jurgenhaas Gottmadingen
I've made a couple of clean-ups to the GinContentFormHelper class and can report that everything seems to be working without a block or allow list. Here is my test series:
- Content and non-content forms
- Settings form with included ajax requests, e.g. changing field settings by increasing the cardinality
- Modal forms like e.g. in views
-
๐
Problem since rc11 with Action buttons
Postponed: needs info
: it now works, just that the "Next" action button is in the hamburger menu, but that should be solved in multi_setp by adding
#gin_action_item = FALSE
to their action button - ๐ The save button is missing for the media directories forms. Active : it works for the reported media directories form. There is a remaining issue with their media directories add form because that builds a popup over the modal which also contains a form, but that's been done in an ajax wrapper instead of a drupal_modal wrapper. If that were corrected, even that one would work out of the box. I'm leaving a comment for that in the other issue.
What's essentially changed is that Gin action button processing is disabled for
drupal_modal
anddrupal_dialog
only, but it still works on simple ajax calls. That results in updated form IDs for rebuilds to also update the sticky action buttons, which is really nice.So, as far as I can tell, things are working really well. Next steps should be:
- More tests by other people.
- Clarifying if media_directories could change their second level modal into a drupal_modul instead of an ajax wrapper
- Going back to Gin's code and verifying if any of the exception handlings could now be removed
- Status changed to Needs work
9 days ago 3:50pm 29 August 2024 - ๐ฉ๐ชGermany jurgenhaas Gottmadingen
Just found 1 popup in views, which doesn't work yet. While apparently all popups in the views UI work as expected, there is one exception to that: re-arranging fields/filters/relationships/etc. - those are missing the action buttons. That looks like a similar issue to what we see with the second popup in media directories. Maybe that helps to identify a pattern to even address that in a generic way. I'll have another look.
- Status changed to Needs review
9 days ago 7:29am 30 August 2024 - ๐ฉ๐ชGermany jurgenhaas Gottmadingen
It was the
views_ui_rearrange_
form ID that needed to be added to\Drupal\gin\GinContentFormHelper::stickyActionButtons
. Back to NR. - ๐ฉ๐ชGermany jurgenhaas Gottmadingen
I've tested the new approach with all the cases in #18. Most use cases look really good.
Just found 2 issues:
- When going through the multi-step form, I'm seeing warnings like these:
Warning: Undefined array key "#type" in gin_form_after_build() (line 65 of themes/contrib/gin/includes/form.theme).
- When using the media directories module, the main dialog works, but adding new media is showing with action buttons.
Other than those edge-cases, I haven't found any other issues. Well done @saschaeggi !!!
- When going through the multi-step form, I'm seeing warnings like these: