Fix handling of onClick event for accessibilityRole="link"

[EyMaddis]

This is a extracted from #1348 in order to separate the discussion of the two issues.
By default onClick and other DOM based props are passed down to the element like divs, but not for links!

This is currently not in the documentation, but used in this example and in the official guides (here: onMouseEnter/Leave) from @necholas!

Using onClick is crucial when avoiding Touchables/ScrollViews in to not inherit issues from #1219. Mobile browsers already solve scroll-based issues if we use regular click events.
This PR aims to allow just that and fix the link handling, as all other RNW elements support the onClick.

This results in problems like:

<View
 onClick={() => console.log('called')}
 accessibilityRole="button"
>

<View
 onClick={() => console.log('never called')}
 accessibilityRole="link" // want to use for accessibility and SEO reasons
>

I hope that you hear me out. Thanks for the great library! 👍

[loudwinston]

This is a great fix! Thanks @EyMaddis !

Any chance that this can get some attention from project maintainers @necolas ? We're having to fork react-native-web to apply this patch for our project. It'd be great to include this in the official version.

[necolas]

Why are you using onClick at all?

[loudwinston]

@Neclos we are using onClick to get around an issue with onPress where scrolling in a mobile brower sometimes triggers the onPress event.

I believe there's an issue for this specifically but I'm on mobile at the moment so having trouble finding it

[EyMaddis]

Hi @necolas, thanks for taking a look! :)

Like @loudwinston, I avoid ScrollViews and Touchable. I just had too many issues, and want an app that feels native to the web. By that I do not mean an iOS/Android app that also runs on the browser, but a fully fledged "regular" web app.
This also includes body scrolling and how Chrome/Safari handle touch on clickable elements.

In addition to that, the app is also a Smart TV app which triggers click events when using the "OK" button on the remote control.
The main issue is with responder system, but I don't see that getting fixed in the few weeks before release.

I did try to do it properly, e.g. I actually have a body scroller prototype locally, but it does not solve the broader issues.

Generally my opinion is that RNW enhances the web platform and should still allow the web apis to be usable. So while this might seem like a hack, I sincerely do not feel that way for the click event since it should be exposed anyway. I find it extremely irritating that the click handling depends on what element is used. If onClick is disallowed, it should not work at all (which I hope never happens!) and not for a specific case with a "random" e.preventDefault().

Thanks

[loudwinston]

@EyMaddis @necolaa to clarify, the application I am working on does use Touchables and ScrollViews. We had to start using onClick to avoid inadvertently triggering touch events on Touchables in ScrollViews when scrolling on the web. We had to fork RNW to remove this e.preventDefault behavior because we use accessibilityRole="link" extensively to ensure screen readers (iOS Voiceover and Android TalkBack) announce our Touchables as links for users that navigate the web version of our app.

There's some refactoring we want to do to make these Touchables into actual links on the web (and perhaps remove the ScrollViews as well), but the app started as a native app and was modified to also work on the web at a later time, so a lot of the code is very native focused at this time.

It would be preferable if we didn't have to fork RNW to make this behavior work for us. I don't know the details of the changes that are being made to the responder system or how this onClick change would impact them, but perhaps this is a small change that could be introduced in the current version to while the responder system changes are still in progress?

[EyMaddis]

@necolas any chance you can take another look at this? :)
I need to deploy a fork of RNW right now for a rather trivial fix.

[necolas]

None of this is stable, so relying on onClick is not a good idea. For example, React Native recently added an onClick prop to View for keyboards, which has different semantics to onClick on the web.

[EyMaddis]

@necolas
Ok, interesting and I understand where this is coming from.

However right now, there is no other way to fix the issues. It seems that regular views are always part of the responder system so it seems to be impossible to use their functionality.

I do not see the responder system getting fixed properly in the next few weeks. I tried, but quickly noticed that this is a deep dive to reacts internals.
All that in order to let react native web reimplement the click handling that mobile browsers include natively anyway.

On the other hand, the lines right after that fix officially support and use onClick for button elements, however links are blocked:

react-native-web/packages/react-native-web/src/exports/createElement/index.js

Lines 86 to 96 in fb93019

	if (isButtonLikeRole && !isDisabled) {
	domProps.onKeyPress = function(e) {
	if (!e.isDefaultPrevented() && (e.which === 13 || e.which === 32)) {
	e.preventDefault();
	if (onClick) {
	onClick(e);
	}
	}
	};
	}
	};

I agree with the problem that react native introduces via its own onClick, but for me this does not invalidate this approach at all.

[EyMaddis]

@loudwinston I now use https://www.npmjs.com/package/patch-package with the following:

You might use another version of RNW, if follow the instructions from patch-package and manually fix it in your project

diff --git a/node_modules/react-native-web/dist/exports/createElement/index.js b/node_modules/react-native-web/dist/exports/createElement/index.js
index 903f855..8152350 100644
--- a/node_modules/react-native-web/dist/exports/createElement/index.js
+++ b/node_modules/react-native-web/dist/exports/createElement/index.js
@@ -72,7 +72,7 @@ var adjustProps = function adjustProps(domProps) {
   // preceding mouse events in the responder system.
 
   if (isLinkRole && onResponderRelease) {
-    domProps.onClick = function (e) {
+    domProps.onClick = onClick || function (e) {
       if (!e.isDefaultPrevented() && !isModifiedEvent(e.nativeEvent) && !domProps.target) {
         e.preventDefault();
       }

[necolas]

0.13 resolves all the issues that using onClick was presented as a use-case for. And the issues with links and onClick is also resolved.