Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The href='#!' attribute in the Emoji class is triggering a full app re-render with react router 4 #50

Closed
bnenu opened this issue Jan 23, 2018 · 7 comments
Labels

Comments

@bnenu
Copy link

bnenu commented Jan 23, 2018

Clicking on an emoji changes the router location by adding the "#!" which then triggers re-render on all router dependent components (React Router 4.4.0, react-router-dom 4.4.2, emoji-picker-react 1.7.1, react 16)

@ealush
Copy link
Owner

ealush commented Jan 27, 2018

Hey @bnenu, thanks for submitting this issue.
Please ignore everything I wrote below. Found the issue, pushing a fix - both 1.7.x and 2.0.x.

Here's the bug:

To support the diversities picker long-click functionality, I used onMouseDown, and remove the click handler, meaning nothing prevents its default.

This seems to be related to: #38
In 1.6.1 I added the ability to get the event from the Emoji Click.
Can you try calling event.preventDefault() in your onEMojiClick callback event?

@ealush
Copy link
Owner

ealush commented Jan 27, 2018

@bnenu

There you go:
https://github.com/ealush/emoji-picker-react/releases/tag/1.7.2
feel free to update to 1.7.2

npm install --save 1.7.2

Let me know if that works for you.

@ealush
Copy link
Owner

ealush commented Jan 27, 2018

Published 2.0.2 as well.

@bnenu
Copy link
Author

bnenu commented Jan 29, 2018

Hi, 2.0.2 is working now as expected. Many thanks for the quick reply.

I do have one question, is there a special reason you used an 'a' tag with a href attribute for the emoji, as far as I can tell you could have used any element with a 'onclick' handler?

@bnenu bnenu closed this as completed Jan 29, 2018
@ealush
Copy link
Owner

ealush commented Jan 29, 2018

@bnenu

Hi, 2.0.2 is working now as expected. Many thanks for the quick reply.
Hey, no problem. I'm glad that it worked.

I do have one question, is there a special reason you used an 'a' tag with a href attribute for the emoji, as far as I can tell you could have used any element with a 'onclick' handler?

Yes. It is more correct, semantically speaking, to use a clickable elements for clickable parts of your app - even though it could work without it. My mistake was using an anchor tag (<a/>) instead of the more semantically correct <button/> element which is more suitable for non-navigable clickable elements - and wouldn't have caused that bug in the first place.

In the next major (3) I am planning to change the markup and replace all anchors with buttons.

@bnenu
Copy link
Author

bnenu commented Jan 29, 2018

Hey, many thanks for the clarification. As a sidenote, I am not sure if it really breaks anything but in the docs you are mentioning 'emoji-js' for rendering emojis and as far as I could tell they changed their API with latest release(3.x) so the example given might not work. I used another package

  • 'react-emojione'.

@ealush
Copy link
Owner

ealush commented Jan 30, 2018

Hm. I was not aware of this change. I'll look into their new API and if needed, modify the docs accordingly. Thanks!

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

No branches or pull requests

2 participants