-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: Remove outdated test cases #147
Merged
Merged
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
18e2f9d
docs: Remove duplicative Dark Mode Group block test case
dcalhoun 056e799
docs: Remove automated Rich Text cases
dcalhoun 585aa7c
docs: Remove automated List splitting and merging test
dcalhoun 9f248b3
docs: Remove automated undo and redo test cases
dcalhoun 46262fe
docs: Remove automated undo and redo test case
dcalhoun 39092ca
test: Reinstate text highlight without selection
dcalhoun 6440263
docs: Reinstate line break between test cases
dcalhoun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked the referenced automated tests and the test cases of the Paragraph block but couldn't find cases related to highlighting text. Hence, probably we should keep these two as manual until we automate them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. I added an integration test for highlighting with selection in WordPress/gutenberg#52446.
For highlighting without a selection, the
typeInRichText
helper does not support inner HTML tags, only outer HTML tags. Updating it to support inner HTML tags would get complicated really quickly, e.g. how might we handle typing with a text selection that begins within a tag but ends outside of it? Therefore, an e2e test likely makes more sense. I'll reinstate this case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, although I'm wondering if we need support for inner HTML tags for this case. If we just create a Paragraph block, tap the Highlight text button, and type text, I understand we wouldn't need to place the cursor between inner HTML tags in the assertion. Would this be an option? (I haven't tested this myself, so I might be overlooking an issue about inner HTML tags per your comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A
mark
tag is inserted once the Highlight text button is tapped. The next call oftypeInRichText
places the cursor at the end of the input (the expected default). However, the reality is that "the end" is after the closingmark
tag. To counter this we either need to (A) add support for inner HTML tags or (B) manually adjust for the closing mark tag — e.g.targetPosition - lengthOfClosingMarkTag
— which feels brittle and cryptic.Inner HTML tag support would resolve the issue of difference between rendered string position (position in a visual representation after HTML is parsed and applied) vs literal string position (position in a raw string that includes HTML mark up).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, true, I didn't realize that the
mark
tag was inserted upon tapping the button. I understand now the necessity of supporting the inner HTML tag. Thanks David for elaborating 🙇 !I agree that option A seems the best way to go 👍 .
Maybe we could create a follow-up GitHub issue with this, so we can keep track of it. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created WordPress/gutenberg#52520 capturing this enhancement.