Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Allow events to be replaced #151

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

Merged
dmt0 merged 2 commits into plotly:master from gonzofish:master
Oct 17, 2019
Merged

Allow events to be replaced #151

dmt0 merged 2 commits into plotly:master from gonzofish:master
Oct 17, 2019

Conversation

Copy link

@gonzofish gonzofish commented Jul 1, 2019

Addresses #150

  • Moved code for adding an event to addEventHandler
  • Moved code for removing an event to removeEventHandler
  • Created utility function for formatting plotly event names
  • Added 3rd condition to syncEventHandlers that will replace a handler if it is not the already-stored handler

janosh and Nouzbe reacted with thumbs up emoji
Matt Fehskens added 2 commits July 1, 2019 15:31 
- Added test for checking that a handler was added
Copy link

FloChehab commented Jul 16, 2019 •
edited
Loading

Hi, I was having a similar issue as described in #150 yesterday. I just wanted to say that I have tested the code from this PR and it indeed solves the issue in my case.

Thanks ! 😄

Copy link
Author

Glad I could help @FloChehab...my issue became with using hooks, where the callback function that got called was a number of renders ago. My solution ended up using useCallback to keep a memoized reference

Copy link

Can some of the maintainers please take a look? We are facing the same issue, would be nice to be able to update event handlers.
jamiehale, janosh, gonzofish, Nouzbe, and mtadams007 reacted with thumbs up emoji

Copy link
Contributor

@VeraZab @dmt0 can you guys please merge this and release it and then incorporate it into the next build of RCE, which should also include plotly.js 1.50.1?

Copy link
Contributor

@gonzofish thanks for this PR, and my apologies it took so long to get it looked at :)

@dmt0 dmt0 merged commit 441bbfd into plotly:master Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /