[REF] tests: remove mock content editable helper #5494
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The goal is to re-use those functions in the test suite to easily insert new text at the current selection (see next commit)
|
|
LucasLefevre
force-pushed
the
master-remove-content-editable-mock-lul
branch
3 times, most recently
from
e38b25f to
999a226
Compare
We have a class `ContentEditableHelper` which is a helper class designed to manipulate the DOM and the selection of the composer element. It has been mocked since the begining of o-spreadsheet, which means all the business logic inside isn't tested at all! It was mocked because jsdom (the lib implementing/mocking DOM for running tests in nodejs) didn't support the Selection API at the time this project was started. However, the Selection API was implemented back in 2019 and we've updated jsdom since then. `ContentEditableHelper` has grown over the years and it now contains even more business logic which is worth testing. And it's probably meant to grow even more in the future as we add features to the composer (as an example, see task 4471274). This commit removes our mock and we now rely on jsdom implementation. Caveats: - jsdom doesn't update the selection when arrow keys (right or left) are pressed. We have to manual set the selection at the right place. - jsdom doesn't support having the focusNode/offset before the anchorNode/offset (selecting multiple characters from right-to-left). Hence it can now longer be tested and one test had to be removed. Task: 4491759
LucasLefevre
force-pushed
the
master-remove-content-editable-mock-lul
branch
from
999a226 to
4e3cab2
Compare
LucasLefevre
changed the title
LucasLefevre
commented
Jan 17, 2025
| @@ -896,7 +962,7 @@ describe("composer", () => { | |||
| expect(composerStore.currentContent).toEqual("Azerty"); | |||
| expect(composerStore.composerSelection).toEqual({ start: 6, end: 6 }); | |||
|
|
|||
| composerStore.changeComposerCursorSelection(6, 4); | |||
| composerStore.changeComposerCursorSelection(4, 6); | |||
There was a problem hiding this comment.
right-to-left selection isn't supported
LucasLefevre
commented
Jan 17, 2025
| @@ -777,28 +850,20 @@ describe("composer", () => { | |||
| expect(composerStore.editionMode).toBe("inactive"); | |||
| }); | |||
|
|
|||
| test("Select a right-to-left range with the keyboard", async () => { | |||
There was a problem hiding this comment.
right-to-left selection isn't supported
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
We have a class
ContentEditableHelperwhich is a helper class designedto manipulate the DOM and the selection of the composer element.
It has been mocked since the begining of o-spreadsheet, which means all the
business logic inside isn't tested at all!
It was mocked because jsdom (the lib implementing/mocking DOM for running
tests in nodejs) didn't support the Selection API at the time this project
was started.
However, the Selection API was implemented back in 2019 and we've updated
jsdom since then.
ContentEditableHelperhas grown over the years and it now contains even morebusiness logic which is worth testing.
And it's probably meant to grow even more in the future as we add features
to the composer (as an example, see task 4471274).
This commit removes our mock and we now rely on jsdom implementation.
Caveats:
pressed. We have to manual set the selection at the right place.
(selecting multiple characters from right-to-left). Hence it can now longer
be tested and one test had to be removed.
Task: 4491759
review checklist