Skip to content

fix(axis): add axisTitleHeight to axis increments#29

Merged
emmacunningham merged 2 commits intoelastic:masterfrom
emmacunningham:fix/multi-axis-spacing
Feb 5, 2019
Merged

fix(axis): add axisTitleHeight to axis increments#29
emmacunningham merged 2 commits intoelastic:masterfrom
emmacunningham:fix/multi-axis-spacing

Conversation

@emmacunningham
Copy link
Copy Markdown
Contributor

@emmacunningham emmacunningham commented Feb 1, 2019

fixes #26

Previously, without the axisTitleHeight offset, multiple axes with the same position would overflow
into each other with margins less than the title height. Also it would appear to give the chart additional padding because the axis nearest to the chart wasn't positioned correctly.

Now, the title won't overflow into the next axis:

image

(Has been tested with multiple axes for each of the positions)

@markov00
Copy link
Copy Markdown
Collaborator

markov00 commented Feb 4, 2019

@emmacunningham could you please add a story with multiple Y and X axes on top, bottom, left and right with debug knobs so we can add a test for this in the future?

@markov00
Copy link
Copy Markdown
Collaborator

markov00 commented Feb 4, 2019

and maybe some knobs to show and hide the axes

Previously, without the axisTitleHeight offset, multiple axes with the same position would overflow
into each other with margins less than the title height.

fixes elastic#26
@emmacunningham
Copy link
Copy Markdown
Contributor Author

@markov00 Yep, see commit 4607394

There's a toggle to show/hide each of the 8 axes (knobs grouped by position) and also a theme group for knobs to adjust margins/paddings as that might also affect how the axes stack on top of each other:

image

Copy link
Copy Markdown
Collaborator

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

Code LGTM.
I'm not sure if the chart.margin has to be applied outside the chart and also between axes, but for the moment I think it's super fine and we can later add a new props that describe the space between axes

@emmacunningham emmacunningham merged commit e34f0ae into elastic:master Feb 5, 2019
markov00 pushed a commit that referenced this pull request Feb 7, 2019
# 1.0.0 (2019-02-07)

### Bug Fixes

* reflect specs ids on legend items when using single series ([8b39f15](8b39f15))
* **axis:** add axisTitleHeight to axis increments ([#29](#29)) ([e34f0ae](e34f0ae)), closes [#26](#26)
* **axis:** fix horizontal title positioning to account for title padding ([08d1f83](08d1f83))
* **axis:** scale tick labels to fix text truncation on chrome ([#38](#38)) ([99c2332](99c2332)), closes [#18](#18)
* **axis:** use titleFontSize for debug rect for horizontal axis title ([#17](#17)) ([af4aa58](af4aa58)), closes [#11](#11)
* **dimensions:** use chart top padding in computation of chart height ([42585f7](42585f7)), closes [#13](#13)
* **x_domain:** fix x value asc sorting using numbers ([26b33ff](26b33ff))

### Features

* add tickLabelRotation and showGridLines features ([#7](#7)) ([47f118b](47f118b))
* **axis:** draw grid lines separately from axis tick and customize style with config ([#8](#8)) ([ab7e974](ab7e974))
@emmacunningham emmacunningham deleted the fix/multi-axis-spacing branch March 7, 2019 16:02
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
# 1.0.0 (2019-02-07)

### Bug Fixes

* reflect specs ids on legend items when using single series ([ecbe4c6](elastic/elastic-charts@ecbe4c6))
* **axis:** add axisTitleHeight to axis increments ([opensearch-project#29](elastic/elastic-charts#29)) ([1f4b48b](elastic/elastic-charts@1f4b48b)), closes [opensearch-project#26](elastic/elastic-charts#26)
* **axis:** fix horizontal title positioning to account for title padding ([d167af6](elastic/elastic-charts@d167af6))
* **axis:** scale tick labels to fix text truncation on chrome ([opensearch-project#38](elastic/elastic-charts#38)) ([3e6a400](elastic/elastic-charts@3e6a400)), closes [opensearch-project#18](elastic/elastic-charts#18)
* **axis:** use titleFontSize for debug rect for horizontal axis title ([opensearch-project#17](elastic/elastic-charts#17)) ([7e5f7d6](elastic/elastic-charts@7e5f7d6)), closes [#11](elastic/elastic-charts#11)
* **dimensions:** use chart top padding in computation of chart height ([4bc5ac8](elastic/elastic-charts@4bc5ac8)), closes [#13](elastic/elastic-charts#13)
* **x_domain:** fix x value asc sorting using numbers ([f64c6aa](elastic/elastic-charts@f64c6aa))

### Features

* add tickLabelRotation and showGridLines features ([#7](elastic/elastic-charts#7)) ([ced76ce](elastic/elastic-charts@ced76ce))
* **axis:** draw grid lines separately from axis tick and customize style with config ([#8](elastic/elastic-charts#8)) ([411950b](elastic/elastic-charts@411950b))
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.

Fix issues with spacing of multiple axes with same position

2 participants