Skip to content

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
merged 2 commits into from
Oct 17, 2019
Merged

Allow events to be replaced #151

merged 2 commits into from
Oct 17, 2019

Conversation

gonzofish
Copy link

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

Matt Fehskens added 2 commits July 1, 2019 15:31
- Added  test for checking that a handler was added
@FloChehab
Copy link

FloChehab commented Jul 16, 2019

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 ! 😄

@gonzofish
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

@korommatyi
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.

@nicolaskruchten
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?

@nicolaskruchten
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants