-
Notifications
You must be signed in to change notification settings - Fork 185
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
automatic height #337
automatic height #337
Conversation
closes #323
- y is always either ordinal or quantitative - fy is always ordinal
I've revised all the test/plots/ :
(I think that athletes-sport-weight is also a bit too tall—540 would be nicer than 640—, but that's already a manual setting.) |
795fd23
to
86fece1
Compare
closes #323
# Conflicts: # src/plot.js # test/output/caltrain.svg
the large height comes from my solution is to consider that as soon as a chart has vertical facets, the auto height of a facet should be halved: - const nfy = fy ? fy.scale.domain().length : 1;
- return !!(y || fy) * Math.max(1, Math.min(60, ny * nfy)) * 20 + !!fx * 30 + 60;
+ const nfy = fy ? fy.scale.domain().length : 2;
+ return !!(y || fy) * Math.max(1, Math.min(60, ny * nfy)) * 10 + !!fx * 30 + 60; In the test plots this change impacts only: ballotStatusRace, mobyDickFaceted, penguinMassSex and penguinMassSexSpecies. |
…lf that of a usual plot
I’ve committed a further tweak. I prefer that adding faceting does not affect the natural height of bands, but that if the subplots don’t have an ordinal y scale, that we partition the target height of 400px for each facet, but down to a minimum of ~140px for each facet. |
(post-facto approved) |
closes #323
It's not dramatically affecting any of the test plots (for example test/plots/athletes-sport-sex.js' height was fixed manually at 500px, and becomes 508px with this heuristic).