Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Add onClick handler to NavigationLink #266

Open
swese44 opened this issue Mar 2, 2018 · 2 comments
Open

Add onClick handler to NavigationLink #266

swese44 opened this issue Mar 2, 2018 · 2 comments

Comments

@swese44
Copy link
Contributor

swese44 commented Mar 2, 2018

we didn't want people to abuse links for clickable-only things, but there are many use cases (logging) where we need to handle a click and have the navigation. href is required, and we don't allow inline javascript or # there, so we shouldn't need to worry about abuse

@swese44
Copy link
Contributor Author

swese44 commented Mar 10, 2018

@BrendanMcK do you see any issue adding this? otherwise clients need to add click handlers on divs/spans surrounding links to log an event, requiring role="button" etc. to pass tslint rules.

@BrendanMcK
Copy link
Contributor

BrendanMcK commented Mar 10, 2018

Maybe a better pattern is to have a LinkReporter component in the consuming framework that manages listening for these and logging them, and which has the appropriate tslint-ignore stuffs.

(There are plenty of valid reasons for having a click handler as an ancestor of an interactive element, so this tslint rule has potential for false positives.)

I'd be OK with adding this though; but would prefer is we could get a better name. onClick is too close to meaning "handle the interactivity pls". This is not the same as a Button's onClick. What we really want is something with a more passive name that indicates "btw, I'm going to navigate to the link you gave me, fyi" - ? - even handleLogging or similar might make the intent more clear.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants