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

style(xy): expose isolated point style #2004

Merged
merged 23 commits into from
May 31, 2023

Conversation

markov00
Copy link
Member

@markov00 markov00 commented Mar 23, 2023

Summary

Fix part of #1902 by exposing the style of the isolated points.

Now the previously called orphan data points are renamed to isolated points to follow a more mathematical semantic.

Screenshot 2023-03-27 at 15 14 06

Checklist

  • The proper chart type label has been added (e.g. :xy, :partition)
  • The proper feature labels have been added (e.g. :interactions, :axis)
  • The :theme label has been added and the @elastic/eui-design team has been pinged when there are Theme API changes
  • 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
  • The code has been checked for cross-browser compatibility (Chrome, Firefox, Safari, Edge)
  • Visual changes have been tested with all available themes including dark, light, eui-dark & eui-light

@markov00 markov00 added enhancement New feature or request :styling Styling related issue :xy Bar/Line/Area chart related labels Mar 23, 2023
@markov00
Copy link
Member Author

markov00 commented Apr 3, 2023

buildkite update screenshots

@markov00 markov00 changed the title fix: improve orphan datapoint style style(xy): filled and reduces style of orphan datapoints Apr 5, 2023
@markov00 markov00 mentioned this pull request Apr 11, 2023
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.

Not sure if you were waiting on my review or not, but the changes look good to me.

@markov00
Copy link
Member Author

Not sure if you were waiting on my review or not, but the changes look good to me.

Thanks, no I'm just waiting to test this in Kibana directly. I probably will consider exposing this configuration in the theme and tweaking this in Kibana rather than having this hardcoded

@markov00 markov00 changed the title style(xy): filled and reduces style of orphan datapoints style(xy): expose isolated point style May 3, 2023
@markov00 markov00 requested a review from nickofthyme May 22, 2023 20:17
@markov00 markov00 marked this pull request as ready for review May 22, 2023 20:17
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.

Changes LGTM, tested locally and see no issues other than the one below. I left a few minor code comments in addition.

The point shapes story has a regression in the legend symbols.

Screen Recording 2023-05-25 at 04 57 52 PM

@markov00
Copy link
Member Author

Changes LGTM, tested locally and see no issues other than the one below. I left a few minor code comments in addition.

The point shapes story has a regression in the legend symbols.

Screen Recording 2023-05-25 at 04 57 52 PM

Thank you @nickofthyme , this was actually a bug not previously discovered. It is fixed in 2199a2e

@markov00
Copy link
Member Author

buildkite update screenshots

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.

Thanks, changes LGTM! 🚢

@markov00 markov00 merged commit a8c27ea into elastic:main May 31, 2023
12 checks passed
markov00 added a commit to markov00/elastic-charts that referenced this pull request Jun 5, 2023
This commit exposes the style of the isolated points in the theme (previously called orphan data points)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request :styling Styling related issue :theme :xy Bar/Line/Area chart related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants