-
Notifications
You must be signed in to change notification settings - Fork 13.4k
fix(checkbox, toggle): fire ionFocus and ionBlur #30733
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
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Just a few requests but looks great! Nice work catching & fixing this.
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.
Looking really good, just a few change requests
|
||
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => { | ||
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { | ||
test.describe(title('toggle: ionChange'), () => { |
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.
It looks like there's no ionBlur test for toggle like there is for Checkbox. I think it may be worth adding just in case, but up to you
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 forgot to commit them 🤦♀️ 1a411f0
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.
Sorry, I just saw Brandy had already mentioned this. My bad. Great work!
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.
Too slow @ShaneK!
Co-authored-by: Shane <shane@shanessite.net>
Co-authored-by: Shane <shane@shanessite.net>
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.
Great work! 🚀
Issue number: internal
What is the current behavior?
ionFocus
andionBlur
are not being emitted for checkbox and toggle.What is the new behavior?
onFocus
andonBlur
toHost
onFocus
,onBlur
, andonChange
.ionBlur
since Playwright browsers aren't acting like native browsers when it comes to tabbing.Does this introduce a breaking change?
Other information
Dev build:
8.7.7-dev.11761071592.1d1b804d