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

Fix rangeslider + automargin on side:'top' axes #3329

Merged
merged 3 commits into from
Dec 12, 2018
Merged

Conversation

etpinard
Copy link
Contributor

Completing @alexcjohnson's work started after #3323

  • c5d98e6 fixes the range_slider_multiple baseline. It now uses the correct:
// correct - in rangeslider/helpers.js
tickHeight = (ax.side === 'bottom' && ax._boundingBox.height) || 0;

// incorrect - in rangeslider/draw.js
tickHeight = (axisOpts._boundingBox || {}).height || 0;
  • I was able to reproduce the jasmine test failure on my laptop. The new range_slider_top_axis mock that @alexcjohnson added fit in the dflt margin.t of 100 on my laptop (and pressumably on CI too). Commit 1a9949d increases the axis title font to ensure the top margin is pushed by the tick labels.

alexcjohnson and others added 3 commits December 11, 2018 22:04
... to make margin "push" on all machine. Previously the dflt
    margin.t of 100 was enough to fit the xaxis.title on etpinard's
    laptop and presumably on CI as well.
@alexcjohnson
Copy link
Collaborator

Thanks! 💃

@etpinard etpinard merged commit 7bef64e into master Dec 12, 2018
@etpinard etpinard deleted the rangeslider-top-axis branch December 12, 2018 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants