-
Notifications
You must be signed in to change notification settings - Fork 37
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
STCOM-880 focus-related tests #1628
Conversation
…lio-org/stripes-components into stcom-880-focus-related-tests
Kudos, SonarCloud Quality Gate passed! |
|
||
import Popover from '../Popover'; | ||
import PopoverInteractor from './interactor'; |
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.
Can we remove ./interactor.js
then? Or should we instead replace it with lines 16-24 below? It feels wrong to have both.
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.
The local, BigTest OG interactors might have to stick around for a bit - unless we know for certain that no ui-app level tests OR other interactors depend on them.
pressEscape: ({ perform }) => { | ||
return perform((el) => { | ||
return el.dispatchEvent(new KeyboardEvent('keyup', { bubbles: true, cancelable: true, keyCode: 27 })); | ||
}); | ||
} |
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.
Thank you for this helpfully-named function to the otherwise completely opaque keyboard-up-on-number-27.
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'm super psyched to see these tests turned back on again. I know I turned 'em off in one swell foop, but might consider turning them back on in individual PRs since the "fixes" are quite distinct and, simply, because the fixes can be isolated at the component level, unlike the failing test which had to be turned off in aggregate because failure for any one component dooms the whole suite.
We'll see how splitting the changes goes. There were changes in |
Closing in favor of multiple PR's with better history management... |
React changed its behavior for focus/blur events, so simulating these events programmatically has changed...
focus()
still works, butblur()
does not... as opposed toblur
afocusout
event must be dispatched....see facebook/react#19606 (comment) for the basic details/solution....
will need folio-org/stripes-testing#124