-
Notifications
You must be signed in to change notification settings - Fork 5.4k
chore(22552): refactor modal selector in e2e tests #22668
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
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
ec9557c
to
cecee6e
Compare
You have a few instances of |
e767672
to
27ef82b
Compare
@HowardBraham Thanks a lot, that's a good catch. 👍🏻 |
0f896ae
to
0ba64b0
Compare
Builds ready [ed5ae09]
Page Load Metrics (835 ± 26 ms)
Bundle size diffs
|
if (e.message.includes('<div class="modal__backdrop">')) { | ||
// Wait for the modal to disappear and try again | ||
await driver.wait( | ||
until.elementIsNotPresent(By.css('.modal__backdrop')), |
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.
Similar methodology as #22672, we would wait for modal to be completed removed and then perform clicking actions.
PS: Once we completely migrated modal component, we can remove this await.
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.
LGTM!
Description
Following the suggestion divided from this thread to refactor all selectors for modal in e2e tests. They are now referred by
data-testid
in selector.More specifically speaking, when the modal has been adapted to new component, a
data-testid
will be appended directly to<Modal >
component and selected; when the component is using deprecated modal component, thisdata-testid
will be appended to as a property passing toFadeModal
componentRelated issues
Fixes: #22552
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist