Skip to content

Fixes pie unhover event#407

Merged
mdtusz merged 3 commits into
plotly:masterfrom
bcutrell:fixes_pie_unhover_event
Apr 8, 2016
Merged

Fixes pie unhover event#407
mdtusz merged 3 commits into
plotly:masterfrom
bcutrell:fixes_pie_unhover_event

Conversation

@bcutrell
Copy link
Copy Markdown

@bcutrell bcutrell commented Apr 8, 2016

Mirrors fix for adding pie hover events added here: https://github.com/plotly/plotly.js/pull/150/files

Comment thread src/traces/pie/plot.js Outdated
var Color = require('../../components/color');
var Drawing = require('../../components/drawing');
var svgTextUtils = require('../../lib/svg_text_utils');
var Plotly = require('../../plotly');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have been favoring this pattern:

var Fx = require('../../plots/components/graph_interact')

lately.

@etpinard
Copy link
Copy Markdown
Contributor

etpinard commented Apr 8, 2016

@bcutrell Nice PR! Thanks 🍻

A few things before we this one in:

  • make sure that npm run lint return no errors
  • require Fx directly in pie/plot.js
  • merge the unhover test suite in test/jasmine/tests/unhover_pie_test.js

Thanks again.

@bcutrell
Copy link
Copy Markdown
Author

bcutrell commented Apr 8, 2016

Ah, good call, thanks for the quick feedback

@mdtusz
Copy link
Copy Markdown
Contributor

mdtusz commented Apr 8, 2016

Seems reasonable to me.

💃

@mdtusz mdtusz merged commit f67212e into plotly:master Apr 8, 2016
@mdtusz
Copy link
Copy Markdown
Contributor

mdtusz commented Apr 8, 2016

Thanks for the contribution @bcutrell!

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.

3 participants