-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[Mobile] - E2E tests - Add new helpers #48358
Conversation
Size Change: +2.97 kB (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
Flaky tests detected in c6cb481. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4353437687
|
…he inline appender and the formatting options not showing up when the button gets automatically focused
…ing, openLintSettings, getButtonBlockTextInputAtPosition, addButtonWithInlineAppender and waitForElementToBeDisplayedByXPath helpers
ac47b97
to
4b5f2e2
Compare
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.
This looks great! I left a few thoughts to consider.
return ( | ||
<View | ||
testID={ toolbarAriaLabel } | ||
accessibilityLabel={ toolbarAriaLabel } |
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.
Unlike the web, this element does not appear accessible to screen readers, i.e. accessible
is not enabled. Was the intent to make this selectable or should we remove accessibilityLabel
?
Enabling accessible
would change the interaction with the child elements, e.g. insert block button, undo button. The web has keyboard commands to "step into" an element to select a child. I do not believe mobile devices have a similar mechanism, we would likely have to recreate an experience similar to how selecting a block disables accessible
on the ancestor so that descendent UI is selectable.
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 added these to be able to find them for E2E tests, since testID
is not supported on Android we also have to add accessibilityLabel
so it'd be for internal usage only.
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.
testID
is not supported on Android we also have to addaccessibilityLabel
Aha. That is very helpful context. Thank you for explaining that. That is unfortunate. 😞 Is it worth leaving an inline comment stating it is currently only used for testing purposes?
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.
Is it worth leaving an inline comment stating it is currently only used for testing purposes?
I know we have other cases like these around the codebase, so I'm not sure if having several comments explaining why would be the best approach. I feel like if it's next to a testID
it could be self-explanatory once you know it is needed for E2E tests.
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.
Well done! 👏🏻
Related PRs:
What?
This PR adds new helpers for E2E tests and it also fixes an issue where sometimes the formatting tools don't show up correctly when focusing
Button
blocks.Why?
To simplify and avoid duplicating code some helpers were needed.
How?
It adds the following helpers:
toggleFormatting
-> To be able to toggle text formatting e.g.Bold
,Link
.getButtonBlockTextInputAtPosition
-> To get a TextInput element within a Button block.addButtonWithInlineAppender
-> To be able to add new Button blocks within a Buttons block using the in-line appender.selectTextFromTextInput
-> Util to select a text within TextInput.It also updates
waitForKeyboardToBeHidden
to take into account when the device is in Lanscape mode for iOS.Testing Instructions
CI checks should pass on both Gutenberg and Gutenberg Mobile.
Testing Instructions for Keyboard
N/A
Screenshots or screencast
N/A