[HOLD for RN-Modal to support RN 0.65] [Windows only] Cntrl + click doesn't open the link in a new tab for MenuItem component.

[rushatgabhane]

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

---

Expected Result:

Both click and Cltr + click on MenuItem with a link opens the link in new tab.

Actual Result:

Only click opens the link in new tab.
Cltr + click doesn't do anything.

The video below demonstrates that only the component MenuItem (with a link) has this problem.
Text links work as expected.

Expensify.cash.-.Google.Chrome.2021-06-09.02-23-53.mp4

Action Performed:

1. Open settings menu
2. Go to about
3. Cltr + click on "View the code"

Workaround:

User can click without pressing Cltr.

Platform:

Where is this issue occurring?

OS: Windows
Browser: Chrome

Web ✔
iOS
Android
Desktop App ❔
Mobile Web

Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/166732
Upwork job link: https://www.upwork.com/jobs/~01e82d35df4dd24853

View all open jobs on Upwork

[MelvinBot]

Triggered auto assignment to @michaelhaxhiu (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[michaelhaxhiu]

hmm, @rushatgabhane this works for me on Mac when I do a cmd + click (web and desktop app). Is it possible this is just impacting Windows users?

[rushatgabhane]

hmm, @rushatgabhane this works for me on Mac when I do a cmd + click (web and desktop app). Is it possible this is just impacting Windows users?

Yes, looks like it's an issue for windows and linux only. I was able to reproduce the issue on 2 devices.

[Julesssss]

it's certainly minor, but I don't see why we shouldn't fix this for consistency. It would annoy me if this occurred on Mac.

[michaelhaxhiu]

Cool, thanks for the bud check @Julesssss.

I'll post it on Upwork and confirm here when that is done. If you'd like to solve it, you can post a solution proposal @rushatgabhane (in which case, you'd be compensated a bonus for finding this enhancement + solving it)

Then, a member of our eng team will review your proposal and give the 👍 or 👎 on proceeding with a PR.

[michaelhaxhiu]

Ok, this job is posted to Upwork here - https://www.upwork.com/jobs/~01e82d35df4dd24853

@rushatgabhane feel free to apply for the job in Upwork if you want to propose a solution, and you'll receive a $150 bonus in addition to payment for the solution.

[rushatgabhane]

@michaelhaxhiu sure! I'm working on a proposal, will apply soon.

[MelvinBot]

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

[michaelhaxhiu]

@Jag96 will be able to assist with reviewing your proposal as the next step. After we have the 👍 we will assign you to the GH and hire you on Upwork.

[rushatgabhane]

This issue occurs whenever react-native's Linking is used within a Button, Pressable etc. Doesn't happen for Text.
This is reproducible outside E.cash, and that makes me think it's a problem with react-native.

[michaelhaxhiu]

This is reproducible outside E.cash, and that makes me think it's a problem with react-native.

Oh wow interesting~ In that case, I feel like we should stick with the React-native standard and wait on them to fix it. It seems minor to me, especially since there's a solid workaround.

@Jag96 lmk if you agree with this angle or want to continue with fixing this.

Either way @rushatgabhane we appreciate you filing this! We love when contributors discover and file jobs. Keep it up!

[mallenexpensify]

Add Planning label since it's on hold, to help with GH searching by label

[MelvinBot]

@Jag96, @rushatgabhane it looks like no one is assigned to work on this job.
Please double the price or add a comment stating why the job isn't being doubled.

[rushatgabhane]

Issue is on hold.
Ignore the bot.

[Jag96]

Still on hold

[kadiealexander]

Please refer to this post for updated information on the n6 hold, we've raised the bonus to $250 for all issues where a PR is created before the N6 hold is lifted.

[Jag96]

Still on hold

[kidroca]

Overall I agree with what @parasharrajat is suggesting here: #3455 (comment)

---

The semantic and accessibility compliant way is to render an anchor tag
There are 2 types of links

• internal -> links that take you somewhere else inside the app
• external -> links that take you to an external page

Internal Links

We're not handling internal routing as links, but react to button presses (simulating link behavior)
There's nothing wrong with that, but it means some of the default context actions, like opening a LHN conversation in a new tab, aren't possible
React Navigation provides a guide on how to solve this problem for internal links if we ever reconsider: https://reactnavigation.org/docs/link/

The Link component lets us navigate to a screen using a path instead of a screen name based on the linking options. It preserves the default behavior of anchor tags in the browser such as Right click -> Open link in new tab", Ctrl+Click/⌘+Click etc.
If you want to use your own custom touchable, you can use useLinkProps instead

External Links

It seems our concern for this ticket is regarding external links

From the react-native-web docs we can see that we can make any View or Text be rendered as a link by providing a href prop: https://necolas.github.io/react-native-web/docs/accessibility/#links
We're already doing this with our TextLink component

A MenuItem can act more like a link if we provide a href and add it to the underlying <Pressable>.
This exposes the context menu for links and covers middle click and ctrl clicks
To incorporate this with less changes and less platform specific logic we can prevent the default behavior of an event - e.preventDefault()
This way we get the best of both worlds - context based actions for links and letting our app decided what happens for the onPress handler like calling openExternalLink

Since I've already tried this I've captured a draft of the necessary changes here: #6296

With a bit more changes we could also cover internal links

[Jag96]

@kidroca that's a great breakdown, thanks for having a look at that. Since this is just affecting the control + click for windows users, I still don't think it's worth making all these changes that can affect how links work on all platforms across the app, so I'm leaning towards leaving this on hold.

If anyone has different opinions, feel free to chime in here.

[tgolen]

I'm fine with that stance. Maybe it turns into a bigger issue at some point and we can revisit this proposal.

…

On Mon, Nov 15, 2021 at 4:14 PM Joe Gambino ***@***.***> wrote: @kidroca <https://github.com/kidroca> that's a great breakdown, thanks for having a look at that. Since this is just affecting the control + click for windows users, I still don't think it's worth making all these changes that can affect how links work on all platforms across the app, so I'm leaning towards leaving this on hold. If anyone has different opinions, feel free to chime in here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3455 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJMAB2FG34CKP2BIUPQVDLUMGH5PANCNFSM46K6RMCQ> .

[Jag96]

Still on hold

[parasharrajat]

@Jag96 This should have been resolved now. We upgraded RN-web.

[rushatgabhane]

Wohoo! This issue is resolved and can be closed.
But yeah console errors are a thing now. Because RN-Modal is on 0.61.0

Screencast.from.06-01-22.06_32_54.PM.+03.mp4

[mallenexpensify]

Is any compensation due for this issue? I'll assume no if no one comments

[rushatgabhane]

Hmm, issue reporting bonus? Depends on if it works retroactively.

[mallenexpensify]

@rushatgabhane if you reported a bug or feature request that got fixed internally or externally, and you were the first to report it, I think you're eligible for the reporting bonus (even if it's many months old). Sound good @Jag96 ?

[Jag96]

Yeah that sounds good to me! @rushatgabhane I've created an Upwork job and invited you!

[rushatgabhane]

Done, thank you!

[Jag96]

Paid!