react-router-hash-link icon indicating copy to clipboard operation
react-router-hash-link copied to clipboard

Make it possible to opt out of focus handling

Open Haraldson opened this issue 3 years ago • 3 comments

Possible bonus: Regard contenteditables as interactive elements, but I understand if you don’t want this as part of the same PR, or have considered this before and concluded that it’s best to leave them out.

I’m not entirely sure about the flow of arguments here and whether it could be simplified or flow in a more natural way, but it seems to be working as intended.

Fixes #89

Haraldson avatar Oct 04 '21 21:10 Haraldson

Did a lil’ self-review and noticed my brain did a derp. This code will work as intended in my use case, or for people who explicitly defined the prop, but not for anyone else.

The JavaScript flavor here seems fairly modern, but I don’t see any default values in function signatures, null-coalescing operators or similar. What’s your preferred way of defining default values, @rafgraph?

Haraldson avatar Oct 04 '21 21:10 Haraldson

Thanks for the PR. Generally looks good, thanks for contenteditable addition.

One important change, the preventFocusHandling prop should be tracked as a singleton (like hashFragment on line 5) and not passed to the functions.

rafgraph avatar Oct 07 '21 13:10 rafgraph

I hope and think that should do it, @rafgraph!

Haraldson avatar Oct 07 '21 16:10 Haraldson