Skip to content

Conversation

@dmt0
Copy link
Contributor

@dmt0 dmt0 commented Jun 28, 2018

Addresses #473

@dmt0 dmt0 requested a review from nicolaskruchten June 28, 2018 15:10
@dmt0 dmt0 self-assigned this Jun 28, 2018
@dmt0 dmt0 force-pushed the axes-panel-visibility-issues branch from f49f31a to e5f1f23 Compare June 29, 2018 19:49
@VeraZab
Copy link
Contributor

VeraZab commented Jul 4, 2018

💃

name={_('Hover Projections')}
axisFilter={axis =>
!axis._name.includes('angular') && !axis._name.includes('radial')
!axis._name.includes('angular') &&
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible to have angular in a cartesian subplot? I don't think so... so we can get rid of the first two checks here no?

axisFilter={axis =>
!axis._name.includes('radial') && !axis._name.includes('angular')
!(
axis._name.includes('radial') ||
Copy link
Contributor

Choose a reason for hiding this comment

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

here can we not do something like _subplot.includes('polar') or something?

@dmt0 dmt0 force-pushed the axes-panel-visibility-issues branch 2 times, most recently from 7a72aa0 to d4ae302 Compare July 4, 2018 17:29
@nicolaskruchten
Copy link
Contributor

According to Percy we've just lost the Hover Projections fold for a cartesian set of axes... can you look into that plz?

@dmt0
Copy link
Contributor Author

dmt0 commented Jul 4, 2018

Yep, because subplot is not 'cartesian', but 'xaxis' or 'yaxis'

@nicolaskruchten
Copy link
Contributor

Ah, ok, then that logic needs changing :)

@dmt0 dmt0 force-pushed the axes-panel-visibility-issues branch from d4ae302 to 95891a6 Compare July 4, 2018 19:04
@nicolaskruchten
Copy link
Contributor

💃

@dmt0 dmt0 merged commit d7a9d27 into master Jul 4, 2018
@dmt0 dmt0 deleted the axes-panel-visibility-issues branch July 4, 2018 20:24
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.

4 participants