[HOLD #11768] [$16000] Desktop - The Back ⌘[ and Forward ⌘] shortcuts keys are not working as expected in LHN

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

This issue has been split in two. This one is solely focused on the back/forward actions in LeftHandNav (LHN), the other #8657 is focused on back/forward actions in the main chat window.

Action Performed:

1. Open desktop app
2. Navigate to several conversations
3. Use the back ⌘[ and Forward ⌘] shortcuts

Expected Result:

Back ⌘[ and Forward ⌘] shortcuts should work and navigate through the previously navigated conversations and show correctly in LHN.

Actual Result:

Shortcuts are not working as expected.

Workaround:

User has to manually navigate through the conversations.

Platform:

Where is this issue occurring?

• Desktop App

Version Number: 1.0.82-7

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation
Unable to get a video atm. Will update with video later.
Other issue for back/forward working correctly in the main chat window #8657

Expensify/Expensify Issue URL:

View all open jobs on Upwork

---

From @mallenexpensify https://expensify.slack.com/archives/C01GTK53T8Q/p1628722621224600

The Back ⌘[ and Forward ⌘] shortcuts keys don't work properly on Desktop Version 1.0.82-7

[MelvinBot]

Triggered auto assignment to @joelbettner (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[MelvinBot]

@joelbettner Eep! 4 days overdue now. Issues have feelings too...

[mallenexpensify]

@joelbettner , do you think this can be worked on by a contributor? If so, can you add the External label? Thx

[joelbettner]

Sorry, I've been nose down in allocations. Yes, I think this can be worked on by a contributor.

[MelvinBot]

Triggered auto assignment to @MitchExpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[MitchExpensify]

Upwork job

[parasharrajat]

@MitchExpensify I think this needs Exported label so that an engineer can review proposals.

[MelvinBot]

Triggered auto assignment to @NikkiWines (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[NikkiWines]

@parasharrajat's proposal (Option 1) looks good, @MitchExpensify please hire them for this issue! 🙇

[MitchExpensify]

Hired!

[parasharrajat]

@mallenexpensify Since you faced this issue. I would like to ask about your experience with it. I found out that the app already has these shortcuts in place.

[mallenexpensify]

I don't know why it doesn't work consistently for me on desktop. It's via the keyboard shortcuts and also my back/forth keys on my logitech trackbacll, when don't have issues in other applications, mainly Chrome.
Any thoughts why that might be the case @parasharrajat ?

[parasharrajat]

There was a misunderstanding here. The app is already configured with these shortcuts in main.js so my proposal is invalid now. But, I would like to request @mallenexpensify to provide more concrete reproduction steps or information.

1. When the keys don't work for you?
2. What steps you followed when the keys stop working for you? e.g. went to grab a coffee, petting the cat, and then suddenly...
3. Keys are working but there is someting wrong with the navigation order?

etc.

[parasharrajat]

Thanks, @ahmdshrif for bringing this up. As mentioned by Marc #8841 (comment), we decided to deprecate the current implementation of CustomActions that's why your proposal was not accepted at that time and this issue was put on hold.

we are yet to define a clear plan to solve navigation issues. Other PR will be reverted as well.

[NikkiWines]

Not overdue, issue is on hold

[melvin-bot]

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[NikkiWines]

Not tied to a regression, #10770 is a similar issue and they're discussing closing it in favor of this issue.

[melvin-bot]

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[NikkiWines]

Unassigning myself while these are on hold since there's nothing actionable at this time

[JmillsExpensify]

Snagging myself now that I'm on the tracking issue.

[JmillsExpensify]

See the tracking issue for the latest updates.

[JmillsExpensify]

@marcaaron @puneetlath I wanted to quickly confirm that we still we do commit to doing this in the react navigation project? I think largely the answer should be yes given how standard of a shortcut this is. However, I'm wavering a bit overall on the priority of keyboard actions overall. They all feel best done with a holistic, concerted push rather than piecemeal. Curious for your thoughts!

[melvin-bot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[JmillsExpensify]

Actually....wait a minute. How is this issue not an exact of the older, and more through might I add, issue #8657. I think we should close out this issue as a dupe of that one.

[marcaaron]

I think we can incorporate it into the detailed design doc. My feeling about this issue is that the shortcuts never really worked as "expected" because, well, nobody ever promised that it would work a certain way. There are a lot of misunderstandings about how the memory history implemented in react-navigation works (especially with the drawer navigator in the mix).

There's like 250+ comments I am not going to revisit here, but command + [ is the same thing as a browser back button. So if the browser back button works as expected on web and we switch to a simple stack navigator then I am pretty confident these shortcuts will "just work".

[marcaaron]

Also I think that, yes, the issues are very similar. If we switch to a stack navigator and include chats in the stack then it will be easier to reason about the history and expectations WRT shortcuts like command + [ or command + left arrow.

[ahmdshrif]

the same thing as a browser back button. So if the browser back button works as expected on web and we switch to a simple stack navigator then I am pretty confident these shortcuts will "just work".

@marcaaron same issue on the web. with the back and forward button
when I work on proposals here I debug on the web most of the time. (IDK if you fix something now or not )
also, I should mention that we overwrite the history on navigation custome actions so this is the important part.

[marcaaron]

I should mention that we overwrite the history on navigation custome actions so this is the important part.

Yes I added the code for it so I'm aware how it works 😄

[JmillsExpensify]

Ok cool, appreciate the additional context @marcaaron. So where do we do from here? Give that this is a duplicate of an older issue, I'm still inclined to close this one. The older issue also has better reproduction steps to boot. In fact, I'll go ahead and do so now, though any discussion can keep happening if required.

[michaelhaxhiu]

https://expensify.slack.com/archives/C02NK2DQWUX/p1670871808439109?thread_ts=1668807800.467689&cid=C02NK2DQWUX