fix: all keyboard repeat events for navigation#10322
Conversation
|
Build successful! 🎉 |
LFDanLu
left a comment
LFDanLu
left a comment
There was a problem hiding this comment.
behavior looks good to me. I am slightly worried that there are other areas we missed/may miss in the future so I wonder if allowRepeats=true might be a better as a default? I guess either way we approach it we could miss bad behavior in the future but at least making it default to true makes it match behavior prior to the useKeyboard changes?
I don't feel super strongly either way tbh, just something I was mulling over
| if ('setAnchorDate' in state) { | ||
| state.setAnchorDate(null); | ||
| } | ||
| return false; // TODO: is this really correct? or should it return true when we cancel and only propagate if there's nothing to do |
There was a problem hiding this comment.
my gut feeling is that it should stop propagation if we are cancelling selection and should continue other wise
There was a problem hiding this comment.
ah, that was copied from before, I'll add it to my pr for the keyboard documentation so i don't drop your approval here
3 participants
Closes
found in testing
✅ Pull Request Checklist:
📝 Test Instructions:
Go to components and press and hold on various keyboard keys. If it's navigation related or step related, it should fire/move continuously as long as you are holding the key.
🧢 Your Project: