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(value_labels): zero as valid value for textBorder and borderWidth #1182

Merged
merged 3 commits into from
Jun 3, 2021

Conversation

markov00
Copy link
Member

@markov00 markov00 commented Jun 1, 2021

Summary

We recently introduced a readability enhancement that involves dropping a background opaque border around the value labels. The logic is now fixed to allow the user to hide this border using zero as a valid value in the textBorder or the borderWidth property of the Theme.barSeriesStyle.displayValue.fill theme style.

BREAKING CHANGE: the textBorder of ValueFillDefinition is now optional or a number only

Screen Recording 2021-06-03 at 09 28 33 AM

Checklist

  • Any consumer-facing exports were added to src/index.ts (and stories only import from ../src except for test data & storybook)
  • Unit tests were updated or added to match the most common scenarios

…rWidth

This commit fixes the cases where the user wants to disable the border width.

BREAKING CHANGE: the `textBorder` of `ValueFillDefinition` is now optional or a number only
@markov00 markov00 marked this pull request as ready for review June 1, 2021 15:48
@markov00 markov00 added :annotation Annotation (line, rect, text) related issue :xy Bar/Line/Area chart related labels Jun 1, 2021
Copy link
Contributor

@rshen91 rshen91 left a comment

Choose a reason for hiding this comment

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

Thanks for adding the new snapshot tests LGTM

Copy link
Collaborator

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

LTGM, tested locally

@markov00 markov00 enabled auto-merge (squash) June 3, 2021 15:19
@markov00 markov00 merged commit a64f333 into elastic:master Jun 3, 2021
@markov00 markov00 deleted the 2021_06_01-fix_value_border branch June 3, 2021 15:57
nickofthyme pushed a commit that referenced this pull request Jun 4, 2021
# [30.0.0](v29.2.0...v30.0.0) (2021-06-04)

### Bug Fixes

* **domain:** custom domain should not filter data ([#1181](#1181)) ([76e8dca](76e8dca)), closes [#1129](#1129)
* **value_labels:** zero as a valid value for textBorder and borderWidth ([#1182](#1182)) ([a64f333](a64f333))
* annotation tooltip display when remounting specs ([#1167](#1167)) ([8408600](8408600))
* render nodeLabel formatted text into the nodes ([#1173](#1173)) ([b44bdff](b44bdff))

### Features

* **axis:** allow pixel domain padding for y axes  ([#1145](#1145)) ([7c1fa8e](7c1fa8e))
* apply value formatter to the default legend item label ([#1190](#1190)) ([71474a5](71474a5))
* **tooltip:** stickTo vertical middle of the cursor ([#1163](#1163)) ([380363b](380363b)), closes [#1108](#1108)
* **wordcloud:** click and over events on text ([#1180](#1180)) ([196fb6a](196fb6a)), closes [#1156](#1156)

### BREAKING CHANGES

* **value_labels:** the `textBorder` of `ValueFillDefinition` is now optional or a number only
@nickofthyme
Copy link
Collaborator

🎉 This PR is included in version 30.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nickofthyme nickofthyme added the released Issue released publicly label Jun 4, 2021
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
# [30.0.0](elastic/elastic-charts@v29.2.0...v30.0.0) (2021-06-04)

### Bug Fixes

* **domain:** custom domain should not filter data ([opensearch-project#1181](elastic/elastic-charts#1181)) ([92ba84c](elastic/elastic-charts@92ba84c)), closes [opensearch-project#1129](elastic/elastic-charts#1129)
* **value_labels:** zero as a valid value for textBorder and borderWidth ([#1182](elastic/elastic-charts#1182)) ([880fbf1](elastic/elastic-charts@880fbf1))
* annotation tooltip display when remounting specs ([opensearch-project#1167](elastic/elastic-charts#1167)) ([7163951](elastic/elastic-charts@7163951))
* render nodeLabel formatted text into the nodes ([opensearch-project#1173](elastic/elastic-charts#1173)) ([0de9688](elastic/elastic-charts@0de9688))

### Features

* **axis:** allow pixel domain padding for y axes  ([#1145](elastic/elastic-charts#1145)) ([6787728](elastic/elastic-charts@6787728))
* apply value formatter to the default legend item label ([opensearch-project#1190](elastic/elastic-charts#1190)) ([20108bb](elastic/elastic-charts@20108bb))
* **tooltip:** stickTo vertical middle of the cursor ([#1163](elastic/elastic-charts#1163)) ([b858fb3](elastic/elastic-charts@b858fb3)), closes [opensearch-project#1108](elastic/elastic-charts#1108)
* **wordcloud:** click and over events on text ([opensearch-project#1180](elastic/elastic-charts#1180)) ([adbf341](elastic/elastic-charts@adbf341)), closes [opensearch-project#1156](elastic/elastic-charts#1156)

### BREAKING CHANGES

* **value_labels:** the `textBorder` of `ValueFillDefinition` is now optional or a number only
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:annotation Annotation (line, rect, text) related issue released Issue released publicly :xy Bar/Line/Area chart related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants