User tests: Successful: Unsuccessful:
Pull Request resolves #40641 .
This fixes two issues affecting Codemirror editors inside subform rows during drag reorder:
Please select:
Documentation link for guide.joomla.org:
No documentation changes for guide.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
| Category | ⇒ | JavaScript Repository NPM Change |
| Labels |
Added:
NPM Resource Changed
PR-5.4-dev
|
||
Thanks for the hint @Fedik . The issue turned out to be in how the editor handles its lifecycle during subform row reordering. so the element is actually moved in the dom (not recreated), which triggers disconnectedCallback() and then connectedCallback(). The current code treats this as a real removal and destroys the editor, so when it reconnects a new instance gets created, which causes duplicated editors. so the new fix is to delay the destruction in disconnectedCallback() and only destroy if the element is actually removed, while cancelling that delay if it reconnects ,so the editor instance is preserved.
You're close, but not quite there yet.
The disconnectedCallback() happens many times, but Codemirror cannot keep up with such amount.
Look why changing order with Up/Down button does not cause this issue, and why Draging the row do cause this issue.
@Fedik Okay so the difference is not in the logic used to move the rows, since both drag and up/down use the same switchRowPositions() function. The key difference is how often it is triggered.
With the up/down buttons, switchRowPositions() is called once per click, that causes a single insertBefore() and one disconnectedCallback() / connectedCallback() cycle, which codemirror can handle correctly.
During drag, however, the row is repositioned multiple times during a single interaction, causing switchRowPositions() to be triggered multiple times. This results in multiple dom reorders and hence repeated disconnectedCallback() / connectedCallback() cycles. Because the editor is destroyed on each disconnect, Codemirror ends up being repeatedly destroyed and re initialized in quick succession, which it cannot handle properly.
Correct, you are looking in the right direction π
| Labels |
Added:
Updates Requested
|
||
Hm, no.
Another hint. The fix is actually ~1 line of code.
And it is in a different place.
@Fedik There are tow separate effects happeing during reorder of the rows.
The one line fix you are suggesting is possibly adding a guard in dragenter - if (!row || row.closest('joomla-field-subform') !== that || row === item) return; but this fixes the duplicate codemirror instances issue, however the content loss still remains.
Should this pr remain scoped only to fixing the duplication issue #40641 , with content loss handled as a separate issue/pr,
or they should be addressed together in this same pr?
You can make both fixes here. But add to the PR description so people know what to test.
For joomla 6 will be need additional PR, there is a different version of Codemirror without editor.save();
I think it can be tested now.
System tests are failing.
I looked, it is something about the Update check.
Not relay related to current PR.
System tests are failing.
I looked, it is something about the Update check. Not relay related to current PR.
Ok, I will restart the tests, maybe that helps.
I have tested this item β
successfully on 460e964
I was able to test this successfully. Thanks @adarshdubey03!
For other testers, make sure you Disable Automated Updates if you are installing Joomla 5.4.4 to test this PR via the Download Package Update.zip, otherwise your installation will instantly update to 5.4.5 and you won't be able to test this PR...
I have tested this item β
successfully on 460e964
I was able to test this successfully. Thanks @adarshdubey03!
For other testers, make sure you Disable Automated Updates if you are installing Joomla 5.4.4 to test this PR via the Download Package Update.zip, otherwise your installation will instantly update to 5.4.5 and you won't be able to test this PR...
Sorry the fix is incorrect.
Hint: It even not really on Codemirror side (however it affects Codemirror itself due to how it work).
I give you a time to find a real reason π