[DRAFT] LPD-39592

[NemethNorbert]

This is a draft to discuss the solution

What is this trying to solve?

LPD-39592

Because the implementation of UndoRedo relies on custom events, and we are using the onBlur event of the input fields to create new history steps, there is an edge case issue, where the user clicks on undo right after editing an input field. In this case we handle undo before we create the new history step from the blur event of the field.

How am I fixing it?

I made sure the Undo / Redo button is disabled while we are editing an input field.

How can you verify that it works?

Follow the reproduction steps from the ticket

Why doesn't this include any test?

This is a draft to discuss the possible solution, however this is changing the UI behaviour by disabling the button. PM review is needed as well

/cc @ambrinchaudhary

[liferay-continuous-integration]

CI is automatically triggering the following test suites:

•     ci:test:sf

[liferay-continuous-integration]

✔️ ci:test:sf - 1 out of 1 jobs passed in 4 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 50cb2bb6110c10d4957252ed4578540ab831a8cd

Sender Branch:

Branch Name: LPD-39592
Branch GIT ID: f889d132ec46b86d3b2b043bf1c6d12ce260cbc6

1 out of 1jobs PASSED

1 Successful Jobs:

• test-portal-source-format

For more details click here.

[liferay-continuous-integration]

Jenkins Build:test-portal-source-format#10609

Jenkins Report:jenkins-report.html

Jenkins Suite:sf

Pull Request:liferay-content-management#6088

Testray Routine:EE Pull Request

Testray Build:[master] ci:test:sf - NemethNorbert > liferay-content-management - PR#6088 - 2024-11-12[07:24:02]

Testray Build ID:66443092

Testray Importer:publish-testray-report#40379

[ambrinchaudhary]

Check if this pull also solves journalArticleAutosave.spec.ts:327:1 Undo/Redo buttons work with metadata fields

[NemethNorbert]

Check if this pull also solves journalArticleAutosave.spec.ts:327:1 Undo/Redo buttons work with metadata fields

It does not, unfortunately that test needs to be rewritten, as the issue it fails with cannot be reproduced manually and it was never reproducible.

[ambrinchaudhary]

Check if this pull also solves journalArticleAutosave.spec.ts:327:1 Undo/Redo buttons work with metadata fields

It does not, unfortunately that test needs to be rewritten, as the issue it fails with cannot be reproduced manually and it was never reproducible.

Unfortunately, it doesn't pass :sob.
Anyway I will focus on review the code of this pull....

[ambrinchaudhary]

I have been checking and testing this issue. I do understand what the problem is and that the solution provided "solves" it. However, I am not very happy with this approach for two main reasons:

• It is strange to me enabling/disabling Undo Redo buttons whenever the user is interacting in the Web Content edition. Not sure if this is a good idea from UX perspective (it could be, we may need to ask).
• We are introducing more events and this is making components more unreadable, involving other components from data engine, input localized... it could be just my perception, but I am also worried because it could cause other effects.

we handle undo before we create the new history step from the blur event of the field.

So, I have checked that the handleStoreState is triggered correctly before the handleUndo and handleRedo functions, but should not be "finishing" on time (or the handleUndo and handleRedo fn start). Which makes me thing that events from outside (data engine, input localized) are triggered correctly, and we may need to find a way to "wait" until the state after the blur is correctly saved before triggering Undo or Redo actions, but all inside our UndoRedo.js component.

I've tried with the setTimeout for those fn with unsuccessfully result, so we need to look for different ways. Some ideas that I haven't tested yet:

• There is a Liferay.once() event that we may use. Maybe when the state after blur is saved we can trigger a Liferay.fire() event and wait for the Liferay.once() in the handleUndo and handleRedo functions. But we have to take care of multiple fires (maybe Liferay.detach() just before storing the step?
• Create a lock/unlock system between those functions (handleStoreState vs. handleUndo / handleRedo)

To be honest, I don't know if that would solve or not the issue, but I think we should investigate a little bit more. If not possible, we can go with this approach (if UX/PM are ok). I will work on this in the next days ;-)