use regular DOM patching when unlocking cloned trees #3652
Conversation
Fixes #3647. Fixes #3651. Relates to: #3591. Issue #3647 was caused by LV uploads relying on DOM attributes like data-phx-active-refs="1,2,3" being in the DOM to track uploads. The problem arises when the upload input is inside a form that is locked due to another, unrelated change. The following would happen: 1. User clicks on a button to upload a file 2. A hook calls this.uploadTo(), which triggers a validate event and locks the form 3. The hook also changes another input in ANOTHER form, which also triggers a separate validate event and locks the form 4. The first validate completes, but the attributes are patched to the clone of the form, the real DOM does not contain it. 5. LiveView tries to start uploading, but does not find any active files. This case is special in that the upload input belongs to a separate form (<input form="form-id">), so it's not the upload input's form that is locked. The fix for this is to only try to upload when the closest locked element starting from the upload input is unlocked. There was a separate problem though: LiveView relied on a separate DOM patching mechanism when patching cloned trees that did not fully share the same logic as the default DOMPatch. In this case, it did not merge data-attributes on elements that are ignored (phx-update="ignore" / data-phx-update="ignore"), therefore, the first fix alone would not work. Now, we use the same patching logic for regular DOM patches and element unlocks. This difference in DOM patching logic also caused other issues, notably: * #3591 * #3651
SteffenDE
force-pushed
the
sd-clone-full-patch
branch
from
64287b0 to
29286ff
Compare
|
Wow, awesome. This makes me feel so appreciative that some people can figure this out, seems like it would take me a lifetime! ❤️ I was hopeful this would resolve our more serious issues with some of our fields clearing / LV's state being totally corrupted in difficult to minimize circumstances. |
|
@marcandre thank you! I hoped that this would fix your other issues as well :( You could try to bisect the repo (clone it somewhere and use it as path dependency), then, before testing run |
|
My bad 🤦♂️. When testing I was trying this branch, but I needed to build the assets too. With the assets build, my complex integration test that led to field clearing and LV state corruption is fixed 🎉. Fabulous ❤️ 🕺 Thank you so much @SteffenDE |
|
Oh, I should have mentioned that assets are only updated after merge.. Anyway, very happy to hear! |
Fixes #3647.
Fixes #3651.
Relates to: #3591.
Issue #3647 was caused by LV uploads relying on DOM attributes like
data-phx-active-refs="1,2,3" being in the DOM to track uploads. The problem
arises when the upload input is inside a form that is locked due to another,
unrelated change. The following would happen:
locks the form
triggers a separate validate event and locks the form
clone of the form, the real DOM does not contain it.
This case is special in that the upload input belongs to a separate form
(
<input form="form-id">), so it's not the upload input's form that is locked.The fix for this is to only try to upload when the closest locked element
starting from the upload input is unlocked.
There was a separate problem though: LiveView relied on a separate DOM
patching mechanism when patching cloned trees that did not fully share
the same logic as the default DOMPatch. In this case, it did not merge
data-attributes on elements that are ignored
(
phx-update="ignore" / data-phx-update="ignore"), therefore, the firstfix alone would not work.
Now, we use the same patching logic for regular DOM patches and element
unlocks.
This difference in DOM patching logic also caused other issues, notably: