-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[$500] Focus on edit when keyboard shortcut is open using CTRL+J #29610
Comments
ProposalPlease re-state the problem that we are trying to solve in this issue.When the delete comment modal is open, opening the right-hand navigation put the focus back on the comment edit field. What is the root cause of that problem?When the delete modal is canceled, it will automatically call the
So, in summary, the Right-hand navigation when opened it dismisses any open modals, causing the Delete modal What changes do you think we should make in order to solve the problem?Since I could not find a way to differentiate between the modal being closed by the user, or by Navigation, I decided to check for the active route when the action occurs. So by checking if the active route is actual report using const isReportRoute = Navigation.getActiveRoute().includes(`/r/${props.reportID}`) We can then blur the text input whenever the route is not a report. useEffect(() => {
if(isReportRoute) {
return;
}
textInputRef.current.blur()
}, [isReportRoute])
What alternative solutions did you explore? (Optional)I tried to update the const handleOnDeleteModalCancel = useCallback(() => {
if(!isReportRoute) {
return;
}
InteractionManager.runAfterInteractions(() => textInputRef.current.focus());
}, [isReportRoute]) This didn't work due to the fact that the modal is closed before the route changes. Meaning N/A |
Triggered auto assignment to @anmurali ( |
Job added to Upwork: https://www.upwork.com/jobs/~0141276117edef8487 |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ArekChr ( |
@abdel-h66 @ArekChr just to drop a note here, we shouldn't use |
Does it mean we are dropping the |
@abdel-h66 we have to, most of the time its solving issues because it wraps the callback passed in setTimeout, now it uses a task queue which is not ideal to handle input focus. |
Just dropping a note. InteractionManager is used because the modal hide callback is called too early. In this issue, I'm working to fix it upstream so we don't need it anymore, but it's still a long way to go. If we want to fix it sooner, then we can use the same fix as we did in EmojiPicker |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@ArekChr this is waiting on you to pick a proposal |
I'm confirming that we don't want to use |
@ArekChr In my proposal, the
|
@abdel-h66 understood, thanks! Could you provide links to the files associated with the code changes mentioned in your proposal? You've included snippets of code, but it's challenging to understand the context without the file references. Thank you! |
I just updated the proposal with the line reference from the codebase. |
ProposalPlease re-state the problem that we are trying to solve in this issue.Text input of edit report action menu is focused when keyboard shortcut panel is open using CTRL+J What is the root cause of that problem?When user delete all text inside text input of edit report action and press
the onCancel parameter is What changes do you think we should make in order to solve the problem?The code base already changed : @situchan the in - ReportActionContextMenu.showDeleteModal(props.reportID, props.action, true, deleteDraft, () => InteractionManager.runAfterInteractions(() => textInputRef.current.focus()));
+ ReportActionContextMenu.showDeleteModal(props.reportID, props.action, true, deleteDraft);
remove the callback for web, but since we use the callback in native app we could add a library function something like: Old Proposal, for achive:When RHN / right panel is opened the onyx modal isVisible value will be true and we already have this useEffect based on onyx modal: App/src/pages/home/report/ReportActionItemMessageEdit.js Lines 136 to 138 in 414d591
we could use this useEffect to add this lines of code: if(modal.isVisible || modal.willAlertModalBecomeVisible) {
textInputRef.current.blur();
} before or after and because of this useEffect and
the What alternative solutions did you explore? (Optional)@situchan I have tested it, removing the onCancel callback () => InteractionManager.runAfterInteractions(() => textInputRef.current.focus()) solve the issue, So the real issue is to focus the edit text input after the cancel delete, right? add InputFocus.inputFocusChange(false); before the showDeleteModal:
and move the Modal.willAlertModalBecomeVisible(false); of basemodal when the modal visibility is false in here: App/src/components/Modal/BaseModal.js Line 89 in c7a376a
into hideModal: App/src/components/Modal/BaseModal.js Line 66 in c7a376a
explanation: We must set the But because we set the https://github.com/necolas/react-native-web/blob/a3ea2a0a4fd346a1b32e6cef527878c8e433eef8/packages/react-native-web/src/exports/Modal/ModalFocusTrap.js#L137 Result:macos-web_d.mp4 |
This issue has not been updated in over 14 days. @anmurali, @zanyrenney, @hayata-suenaga, @situchan, @tsa321 eroding to Weekly issue. |
waiting for payment |
Didn't realise if this was waiting on me or @anmurali |
payment summary |
@tsa321 is not hired on the upwork job, can they link their upwork profile please? I can't find them and this job is now closed so we will need to issue a payment separately. |
I've done one of the payments, just waiting on @tsa321 ^ |
Hi @zanyrenney, whenever you are available, can you check if this issue is eligible for reporting bonus? |
no reporting bonus. |
@tsa321 I asked you to apply for the job - please apply on the job posting on upwork. |
Since this is chat focussed bug, I am putting this in #vip-vsb. |
cc @zanyrenney |
Zany has been OOO. She will come back online this week. Please wait a little bit for that 🙇 @tsa321 |
Hi @tsa321 I have just sent you a manual offer for the job, please accept. |
@hayata-suenaga thanks for the message above, appreciate it! <3 |
not overdue! |
Thank you @zanyrenney , I have accepted the offer |
waiting for payment |
payment summary
|
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.3.84.0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1697207528874409
Action Performed:
Expected Result:
App should not keep focus on edit message when we open keyboard shortcut using CTRL+J
Actual Result:
App keeps focus on edit message even when we open keyboard shortcut using CTRL+J
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
windows.chrome.edit.focus.on.CTRL.J.mp4
mac.chrome.CTRL.J.edit.focus.mov
Recording.4992.mp4
MacOS: Desktop
mac.desktop.CTRL.J.edit.focus.mov
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: