Overview

  • Go to the XB interface for an entity that has a field that uses the Media Library widget.
  • In the page data form, make some edits to the field using the media library widget as well as changes to a field using a simpler widget (page title or some other text field FE).
  • Do not publish your changes
  • Switch to editing a component in the layout so the form changes. Note that I can only make this switch if I add a component to the layout... after I add media I'm unable to click a component and have its form appear That may warrant its own issue if it isn't part of the solution here
  • Return to the Page Data form, note that the recently-changed value of the text field is intact, but the changes in the Media Library widget are not.
  • However, if you publish the changes, those recent changes including the not-in-the-form media library widget changes are published.

Proposed resolution

  • Not sure yet, but worth noting this is in the page data form, so it does not use transforms, so it aint that
  • This might be related to the inputs with missing values being ones added via AJAX vs the render array

User interface changes

CommentFileSizeAuthor
#11 widget breaks when removing.mov6.27 MBjessebaker

Issue fork experience_builder-3533289

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bnjmnm created an issue. See original summary.

nagwani’s picture

nagwani CreditAttribution: nagwani at Acquia commented
Issue tags: +beta blocker
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at Acquia commented
Related issues: +#3514900: EntityFormController should load auto-save state if it exists

I think this is a duplicate of #3514900: EntityFormController should load auto-save state if it exists

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at Acquia commented
Title: Page data form values-in-progress not retained for Media Library (and perhaps other) fields » [PP-1] Page data form values-in-progress not retained for Media Library (and perhaps other) fields
Status: Active » Postponed

Postponing on #3514900: EntityFormController should load auto-save state if it exists

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia at Acquia commented
Title: [PP-1] Page data form values-in-progress not retained for Media Library (and perhaps other) fields » Page data form values-in-progress not retained for Media Library (and perhaps other) fields
Status: Postponed » Active

#3514900: EntityFormController should load auto-save state if it exists is in. Is this issue now fixed as well, or is there more to do here that the other one didn't cover?

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia at Acquia commented
Component: … to be triaged » Redux-integrated field widgets
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at Acquia commented
Assigned: Unassigned » larowlan

Confirming this still occurs after #3514900: EntityFormController should load auto-save state if it exists - I think it is now a FE issue. Will investigate.

larowlan opened merge request !1232

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at Acquia commented
Status: Active » Needs review
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at Acquia commented
Assigned: larowlan » Unassigned
jessebaker’s picture

jessebaker CreditAttribution: jessebaker at Acquia commented
Status: Needs review » Needs work
StatusFileSize
new widget breaks when removing.mov6.27 MB

I have been testing this and the issue described is fixed, at least partially. When returning to the Page Data tab the selected image is correctly displayed. However after leaving and returning to the page data tab, the remove image button no longer functions correctly as the entire widget is removed from the DOM (as shown in attached video).

Furthermore, the same issue described in the IS is also present for media widgets in the component instance form (leaving the form and returning means the selected image is no longer shown).

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Assigned: Unassigned » larowlan
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at Acquia commented
Status: Needs work » Active

Furthermore, the same issue described in the IS is also present for media widgets in the component instance form (leaving the form and returning means the selected image is no longer shown).

I think this means we're going to have to mutate the slice after AJAX calls, its ick but with the requirement to 'make existing Drupal forms just work' we're stuck between a rock and a hard place.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at Acquia commented
Assigned: larowlan » Unassigned
Status: Active » Needs review

Came up with a different approach.

Rather than mutate the form HTML in the slice for both the component and page data forms, instead invalidate them on umount if the form_build_id was changed during rendering. This means any ajax operations that update the form trigger an invalidation of the cache when the form is unmounted.

Seems to work in manual testing and was able to write an e2e test to show it is working, switching tabs doesn't clobber things.

So if you're just switching tabs we keep the cached form around and its snappy.

If you make a change to the form that triggers something AJAX-y the next time we mount we make sure to fetch a fresh form because the cached one is stale and doesn't reflect the new state

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at Acquia commented
Status: Needs review » Needs work

This reveals a fail in multi value widget handling
But it looks as though that exists in HEAD too, the error is that dragging and dropping doesn't trigger an update to the server and autosave

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at Acquia commented
Assigned: Unassigned » larowlan

Back on this

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at Acquia commented
Assigned: larowlan » Unassigned
Status: Needs work » Active
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at Acquia commented
Status: Active » Needs work

Pushed this forward a bit but still failing on multi-value widget

bnjmnm opened merge request !1258

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented

I added 3533289-if-select-value-change-triggers-react-onchange as a draft MR which approaches the tabledrag issue differently - any programmatic changes to the select element are relayed to the onChange callback.

As noted in the MR, if my changes are accompanied by me commenting away the useEffects added to DummyPropsEditForm and PageDataForm, the widget-multivalue.cy.js test is able to get several scenarios further, though it still eventually fails. Although it's not a complete solution, my hopes it is helps surface info that helps us get there.

Also worth noting
I was running into other problems with widget-multivalue.cy.js until I removed the existing components from the layout. More info is in a comment within the test.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at Acquia commented
Status: Needs work » Needs review

Went back to the start on this one.

Started by splitting #3535096: Reordering items in a multi-value field doesn't update auto-save unless the user makes another edit for the existing issue with drag and drop in HEAD as the priority of that is lower than the bug here.

Then dropping down to the root of the issue, it is that switching tabs in the side panel causes the form to umount.
This means any AJAX modifications to the form are lost.
And then when we switch back, we restore the form from the cache in the slice, which doesn't have the ajax modifications.

So instead of trying to work out when AJAX modifications occur, I looked into the tabset component and realised we can prevent the component from being unmounted with the forceMount option.

And this was the sauce.

We still need to trigger invalidations when the component is unmounted, and that is when we use full page preview or the code editor, so I added code for that.

I then tried to reproduce this with the components input form and could not.

I think the difference with that is we have the query params that tracks the current values of the props when using the slice cache and as a result the form is already being correctly invalidated when we need it to. I added additional matching tests for the component inputs form to verify this and they passed without any changes in HEAD.

So the new changeset is much smaller:

  • We make sure the page data form remains mounted when switching between tabs
  • We make sure the page data form cache is invalidated when we come back from preview or code editor
  • Test coverage for all of the above.

The issue with tabledrag is split to #3535096: Reordering items in a multi-value field doesn't update auto-save unless the user makes another edit

larowlan changed the visibility of the branch 3533289-if-select-value-change-triggers-react-onchange to hidden.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at Acquia commented

This is now passing and the changes are much smaller than previous iterations 🎉

  • bnjmnm committed e0eca9d3 on 0.x authored by larowlan 
    Issue #3533289 by larowlan, bnjmnm, jessebaker, lauriii: Page data form...
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented
Status: Needs review » Fixed

glad the solution wound up being so elegant! Merged.

neha_bawankar’s picture

neha_bawankar CreditAttribution: neha_bawankar at Acquia commented

Tested changes on branch 0.x , following scenarios :

Scenario

Result

Status
  • Add image and heading component to a node
  • Add media to in page data tab , and change title
  • Select , heading from layers and edit heading
  • Go back to page data section and check added media and title change are visible
  • Preview Node and then publish node
  • Edited title and media is available : PASS
  • Enable to type in Alternative Text box - FAIL
  • Publish successful
 PASS(ticket for failing scenario : While updating the image, not able to edit/update the Alternative text of the new image from Page Data)
  • Add image and heading component to a node
  • Add media to in page data tab ,and change title
  • Select ,heading from layers and edit heading
  • Go back to image component and check added media is still available
  • Preview Node and then publish node
Media is present PASS
  • Add image and heading component to a node
  • Add media to in page data tab ,and change title
  • Select ,heading from layers and edit heading
  • Go back to page data section and Remove media , add a new media for it
  • Select ,heading from layers and again go back to page data page , check edited media is available
  • Preview Node and then publish node
Edited media is available PASS
  • Add image and heading component to a node
  • Add media to in page data tab ,and change title
  • Select ,heading from layers and edit heading
  • Go back to page data section and Remove media
  • Select ,heading from layers and again go back to page data page ,check edited media is not available
  • Preview Node and then publish node
Removed Media is not available PASS
  • Add image and heading component to a page
  • Add media to in page data tab ,and change title
  • Select ,heading from layers and edit heading
  • Go back to page data section and Remove media
  • Select ,heading from layers and again go back to page data page ,check edited media is available
  • Preview Node and then publish page
Added media is available and publish was successful PASS
  • Add image and heading component to a page
  • Add new Tags (Add new tag in Structure → Taxonomy term → Tags → List Terms → Add new) , select the added tag on xb ui
  • Select ,heading from layers
  • Go back to page data section and Remove media
  • Go back to page data section and check added tag is available
  • Preview Node and then publish node
Added tag is available and publish was successful PASS
  • Add image and heading component to a page
  • Under menu settings ,check Provide a menu link , give a menu title
  • Select ,heading from layers
  • Go back to page data section and check menu setting is saved
  • Preview Node and then publish node
Menu settings saved and publish was successful PASS
  • Add image and heading component to a page
  • Change Author of the page
  • Select ,heading from layers
  • Go back to page data section and check author is changed
  • Preview Node and then publish node
Author changed , Publish was successful PASS
  • Add image and heading component to a page
  • Change state of node from draft to published
  • Select ,heading from layers
  • Go back to page data section and check node state is published
  • Preview Node and then publish node
Node published and publish was successful PASS

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.