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.

Sign up for GitHub

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

Jump to bottom

fix: point style accessor for isolated points #2464

Merged
merged 8 commits into from
Jun 12, 2024
Merged

fix: point style accessor for isolated points #2464

merged 8 commits into from
Jun 12, 2024

Conversation

nickofthyme
Copy link
Collaborator

@nickofthyme nickofthyme commented Jun 11, 2024 •
edited
Loading

Summary

Fixes bug related to using the pointStyleAccessor to override point style on isolated points.

Details

The main issue here is the due to the same underlying cause of #2277 being that isolated styles have a higher priority that all other styles. In #2277 we fixed this when using the (area|line)SeriesStyles but not for pointStyleAccessor.

These changes add the pointStyleAccessor as overrides to any isolated point styles so that the pointStyleAccessor are the highest priority. I also added the isolatedPoint boolean to the PointStyleAccessor arguments to apply styles accordingly if need be.

Issues

fix #2460

Checklist

  • The proper chart type label has been added (e.g. :xy, :partition)
  • The proper feature labels have been added (e.g. :interactions, :axis)
  • All related issues have been linked (i.e. closes #123, fixes #123)
  • New public API exports have been added to packages/charts/src/index.ts
  • Unit tests have been added or updated to match the most common scenarios
  • The proper documentation and/or storybook story has been added or updated

@nickofthyme nickofthyme added :styling Styling related issue :xy Bar/Line/Area chart related labels Jun 12, 2024
@nickofthyme nickofthyme marked this pull request as ready for review June 12, 2024 00:43
@nickofthyme nickofthyme mentioned this pull request Jun 12, 2024
Closed
markov00
markov00 approved these changes Jun 12, 2024
View reviewed changes
Copy link
Member

@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.

Changes looks good to me, thanks!

@nickofthyme nickofthyme merged commit ee7f529 into elastic:main Jun 12, 2024
14 checks passed
@nickofthyme nickofthyme deleted the fix-iso-point-styles branch June 12, 2024 22:22
nickofthyme pushed a commit that referenced this pull request Jun 12, 2024
@semantic-release-bot
chore(release): 66.0.1 [skip ci]
1ee0d1d 
## [66.0.1](v66.0.0...v66.0.1) (2024-06-12)

### Bug Fixes

* point style accessor for isolated points ([#2464](#2464)) ([ee7f529](ee7f529))

### Reverts

* github issue template changes ([501781b](501781b))
@nickofthyme nickofthyme mentioned this pull request Jun 18, 2024
Merged
1 task
nickofthyme added a commit to elastic/kibana that referenced this pull request Jun 21, 2024
@renovate @markov00 @nickofthyme
Update dependency @elastic/charts to v66 (main) (#185906)
e969602 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@elastic/charts](https://togithub.com/elastic/elastic-charts) |
[`65.2.0` ->
`66.0.3`](https://renovatebot.com/diffs/npm/@elastic%2fcharts/65.2.0/66.0.3)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@elastic%2fcharts/66.0.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@elastic%2fcharts/66.0.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@elastic%2fcharts/65.2.0/66.0.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@elastic%2fcharts/65.2.0/66.0.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>elastic/elastic-charts (@&#8203;elastic/charts)</summary>

###
[`v66.0.3`](https://github.com/elastic/elastic-charts/blob/HEAD/CHANGELOG.md#6603-2024-06-18)

[Compare
Source](https://togithub.com/elastic/elastic-charts/compare/v66.0.0...v66.0.3)

##### Bug Fixes

- correct median copy in legend values
([#2467](elastic/elastic-charts#2467))
([0476a64](elastic/elastic-charts@0476a64))

###
[`v66.0.2`](https://github.com/elastic/elastic-charts/blob/HEAD/CHANGELOG.md#6602-2024-06-12)

[Compare
Source](https://togithub.com/elastic/elastic-charts/compare/v66.0.0...v66.0.2)

##### Features

- point style accessor for isolated points
([#2464](elastic/elastic-charts#2464))
([ee7f529](elastic/elastic-charts@ee7f529))

###
[`v66.0.0`](https://togithub.com/elastic/elastic-charts/blob/HEAD/CHANGELOG.md#6600-2024-06-10)

[Compare
Source](https://togithub.com/elastic/elastic-charts/compare/v65.2.0...v66.0.0)

##### Features

- **Metric:** style enhancements
([#&#8203;2437](https://togithub.com/elastic/elastic-charts/issues/2437))
([0686596](https://togithub.com/elastic/elastic-charts/commit/0686596b44fd9cac00955478179597c5b4bd9d91))

##### Performance Improvements

- replace spread with concat where useful
([#&#8203;2446](https://togithub.com/elastic/elastic-charts/issues/2446))
([078b490](https://togithub.com/elastic/elastic-charts/commit/078b4905131fcf368a25bcadcb53e414d634daea))

##### BREAKING CHANGES

- **Metric:** The `MetricStyle.text.darkColor` and
`MetricStyle.text.lightColor` are now `MetricStyle.textDarkColor` and
`MetricStyle.textLightColor`, respectively. This PR also includes minor
overall style changes to the text breakpoints of the `Metric` chart.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/elastic/kibana).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zOTMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjM5My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJUZWFtOlZpc3VhbGl6YXRpb25zIiwiYmFja3BvcnQ6c2tpcCIsInJlbGVhc2Vfbm90ZTpza2lwIl19-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
Co-authored-by: nickofthyme <nicholas.partridge@elastic.co>
bhapas pushed a commit to bhapas/kibana that referenced this pull request Jun 24, 2024
@renovate @markov00
@nickofthyme @bhapas
Update dependency @elastic/charts to v66 (main) (elastic#185906)
c3b5cae 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@elastic/charts](https://togithub.com/elastic/elastic-charts) |
[`65.2.0` ->
`66.0.3`](https://renovatebot.com/diffs/npm/@elastic%2fcharts/65.2.0/66.0.3)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@elastic%2fcharts/66.0.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@elastic%2fcharts/66.0.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@elastic%2fcharts/65.2.0/66.0.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@elastic%2fcharts/65.2.0/66.0.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>elastic/elastic-charts (@&elastic#8203;elastic/charts)</summary>

###
[`v66.0.3`](https://github.com/elastic/elastic-charts/blob/HEAD/CHANGELOG.md#6603-2024-06-18)

[Compare
Source](https://togithub.com/elastic/elastic-charts/compare/v66.0.0...v66.0.3)

##### Bug Fixes

- correct median copy in legend values
([elastic#2467](elastic/elastic-charts#2467))
([0476a64](elastic/elastic-charts@0476a64))

###
[`v66.0.2`](https://github.com/elastic/elastic-charts/blob/HEAD/CHANGELOG.md#6602-2024-06-12)

[Compare
Source](https://togithub.com/elastic/elastic-charts/compare/v66.0.0...v66.0.2)

##### Features

- point style accessor for isolated points
([elastic#2464](elastic/elastic-charts#2464))
([ee7f529](elastic/elastic-charts@ee7f529))

###
[`v66.0.0`](https://togithub.com/elastic/elastic-charts/blob/HEAD/CHANGELOG.md#6600-2024-06-10)

[Compare
Source](https://togithub.com/elastic/elastic-charts/compare/v65.2.0...v66.0.0)

##### Features

- **Metric:** style enhancements
([#&elastic#8203;2437](https://togithub.com/elastic/elastic-charts/issues/2437))
([0686596](https://togithub.com/elastic/elastic-charts/commit/0686596b44fd9cac00955478179597c5b4bd9d91))

##### Performance Improvements

- replace spread with concat where useful
([#&elastic#8203;2446](https://togithub.com/elastic/elastic-charts/issues/2446))
([078b490](https://togithub.com/elastic/elastic-charts/commit/078b4905131fcf368a25bcadcb53e414d634daea))

##### BREAKING CHANGES

- **Metric:** The `MetricStyle.text.darkColor` and
`MetricStyle.text.lightColor` are now `MetricStyle.textDarkColor` and
`MetricStyle.textLightColor`, respectively. This PR also includes minor
overall style changes to the text breakpoints of the `Metric` chart.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/elastic/kibana).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zOTMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjM5My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJUZWFtOlZpc3VhbGl6YXRpb25zIiwiYmFja3BvcnQ6c2tpcCIsInJlbGVhc2Vfbm90ZTpza2lwIl19-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
Co-authored-by: nickofthyme <nicholas.partridge@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markov00 markov00 markov00 approved these changes

Assignees
No one assigned
Labels
:styling Styling related issue :xy Bar/Line/Area chart related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PointStyleAccessor not applying to isolated points
2 participants
@nickofthyme @markov00