-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[HOLD for payment 2022-08-01] [$250] Web - New Room - Drop down is opened again after use the Enter Key to create a new room #9741
Comments
Triggered auto assignment to @MariaHCD ( |
Unable to reproduce on staging or on local on a mac: Untitled_.Jul.7.2022.4_56.PM.mp4But I am able to reproduce the issue on a windows device. |
Triggered auto assignment to @MitchExpensify ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav ( |
Triggered auto assignment to @marcochavezf ( |
Proposal: this.unsubscribe = KeyboardShortcut.subscribe(shortcutConfig.shortcutKey, (e) => {
if (!this.props.isFocused || this.props.isDisabled || this.props.isLoading || (e && e.target.nodeName === 'TEXTAREA')) {
return;
}
+ e.preventDefault();
this.props.onPress();
}, shortcutConfig.descriptionKey, shortcutConfig.modifiers, true, false, this.props.enterKeyEventListenerPriority, false); Before: cinnamon-20220708-10.mp4After: cinnamon-20220708-11.mp4Alternatively, if this somehow breaks usage of validateAndCreatePolicyRoom(e) {
+ e.preventDefault();
if (!this.validate()) {
return;
} |
@mananjadhav, @marcochavezf, @MitchExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@eVoloshchak Went through your proposal and looks like
|
Yes. I checked whatewer seemed logical to check, but I'm not used to navigating a site using keyboard, so I can't check all. In theory it shouldn't break anything, so I think we may add this change to
Keyboard shortcuts are disabled on native, so this listener will never be fired. But then the shorcuts will be added to native apps, it should work fine. |
I tested this on Desktop. I am not able to reproduce this on Mac, but once on Windows. @eVoloshchak's proposal looks good. 🎀 👀 🎀 C+ reviewed cc - @marcochavezf |
Cool, thank you guys! Assigning @eVoloshchak 👍🏽 |
📣 @eVoloshchak You have been assigned to this job by @marcochavezf! |
PR is up |
Issue not reproducible during KI retests. (First week) |
Reminder to pay on August 1st assuming no regressions |
Issue not reproducible during KI retests. (Second week) |
Paid @eVoloshchak, just waiting on @mananjadhav to accept the offer to process C+ payment then we'll close this out |
Done @MitchExpensify |
Paid and contracts ended, thanks everyone! |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
Drop down should not open again after use Enter Key
Actual Result:
Drop down is opened again after use the Enter Key to create a new room
Workaround:
Unknown
Platform:
Where is this issue occurring?
Version Number: 1.1.79.17
Reproducible in staging?: Yes
Reproducible in production?: Yes
Email or phone of affected tester (no customers): any expensifail account
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Recording.729.mp4
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
View all open jobs on GitHub
The text was updated successfully, but these errors were encountered: