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

feat(legend): add legend stats (table view) #2426

Merged
merged 7 commits into from
May 28, 2024
Merged

feat(legend): add legend stats (table view) #2426

merged 7 commits into from
May 28, 2024

Conversation

mbondyra
Copy link
Contributor

@mbondyra mbondyra commented May 8, 2024 •
edited

Summary

This PR enables users to add some statistics (e.g., avg, min, max, etc.) for their metrics in the legend. This way they can get their insights faster by easily accessing more information within the chart. All statistics are computed client-side considering the entire dataset range selected.

Screenshot 2024-05-28 at 10 29 56 Screenshot 2024-05-28 at 10 31 13

Details

Story to test it: Legend/Tabular data

Implements table view for legend statistics. The table shows up automatically when user chooses any statistic other than CurrentAndLastValue for xy charts only.

For the charts that don't have the new legend values added, the legacy view is displayed.

Issues

part of #2096

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
  • The code has been checked for cross-browser compatibility (Chrome, Firefox, Safari, Edge)
  • Visual changes have been tested with light and dark themes

@nickofthyme nickofthyme changed the title Feat legends stats feat(legend): stats May 8, 2024
@mbondyra mbondyra force-pushed the feat_legends_stats branch 8 times, most recently from 7c7aa4d to 61da4e8 Compare May 15, 2024 10:34
@mbondyra mbondyra added :legend Legend related issue :xy Bar/Line/Area chart related labels May 15, 2024
@mbondyra mbondyra changed the title feat(legend): stats feat(legend): add table view for legend stats May 16, 2024
@mbondyra mbondyra force-pushed the feat_legends_stats branch 11 times, most recently from f7ced69 to d19292b Compare May 20, 2024 13:30
@mbondyra mbondyra force-pushed the feat_legends_stats branch 6 times, most recently from a29e5cb to 693a688 Compare May 26, 2024 14:14
@mbondyra mbondyra changed the title feat(legend): add table view for legend stats feat(legend): add legend stats (table view) May 27, 2024
@mbondyra mbondyra force-pushed the feat_legends_stats branch 2 times, most recently from 8b10eaa to 6643841 Compare May 27, 2024 20:34
@mbondyra mbondyra marked this pull request as ready for review May 28, 2024 07:33
dej611
dej611 reviewed May 28, 2024
View reviewed changes
Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Left some code reviews on a first pass.

packages/charts/src/common/legend.ts Outdated Show resolved Hide resolved
packages/charts/src/components/legend/label.tsx Outdated Show resolved Hide resolved
packages/charts/src/components/legend/legend.tsx Outdated Show resolved Hide resolved
storybook/stories/legend/17_tabular_data.story.tsx Show resolved Hide resolved
markov00
markov00 reviewed May 28, 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.

Just minor comments, in general it looks great and I haven't found any bug right now. I will keep testing it

packages/charts/src/common/legend.ts Outdated Show resolved Hide resolved
packages/charts/src/common/legend.ts Outdated Show resolved Hide resolved
packages/charts/src/components/legend/label.tsx Outdated Show resolved Hide resolved
packages/charts/src/common/legend.ts Outdated Show resolved Hide resolved
packages/charts/src/components/legend/types.tsx Outdated Show resolved Hide resolved
packages/charts/src/components/legend/legend_table/legend_table_item.tsx Outdated Show resolved Hide resolved
packages/charts/src/components/legend/legend_table/legend_table_item.tsx Outdated Show resolved Hide resolved
packages/charts/src/components/legend/legend_item.tsx Outdated Show resolved Hide resolved
packages/charts/src/state/selectors/get_legend_table_size.ts Outdated Show resolved Hide resolved
storybook/stories/legend/17_tabular_data.story.tsx Show resolved Hide resolved
@mbondyra mbondyra force-pushed the feat_legends_stats branch 4 times, most recently from ea44e88 to 2c58da4 Compare May 28, 2024 12:34
@mbondyra mbondyra mentioned this pull request May 28, 2024
Open
@mbondyra mbondyra merged commit c22f767 into elastic:main May 28, 2024
14 checks passed
@mbondyra mbondyra deleted the feat_legends_stats branch May 28, 2024 19:36
nickofthyme pushed a commit that referenced this pull request May 28, 2024
@semantic-release-bot
chore(release): 65.1.0 [skip ci]
347d7e8 
# [65.1.0](v65.0.0...v65.1.0) (2024-05-28)

### Bug Fixes

* **deps:** update dependency @elastic/eui to ^94.5.0 ([#2433](#2433)) ([b13ded9](b13ded9))
* **deps:** update dependency @playwright/test to ^1.44.0 ([#2434](#2434)) ([faf36aa](faf36aa))
* **Metric:** should only show one focus halo on `::focus` event ([#2441](#2441)) ([96b0779](96b0779))
* react component type errors ([#2440](#2440)) ([f0b3a00](f0b3a00))

### Features

* **legend:** add legend stats (table view) ([#2426](#2426)) ([c22f767](c22f767))
nickofthyme
nickofthyme reviewed May 29, 2024
View reviewed changes
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.

Excellent job 🎉 🚀

The code looks great and playing around with it works perfectly! I think the way you built it make it easily to merge this with the tooltip table if we decide to go that route.

I just left 2 follow-up things below...

packages/charts/src/components/legend/legend_table/legend_table_item.tsx Show resolved Hide resolved
packages/charts/src/components/legend/legend_table/legend_table.tsx Show resolved Hide resolved
@gvnmagni
Copy link

Adding a little comment here for future considerations, but not a blocker for this specific PR.

  • The horizontal scroll bar should be superimposed on a sort of legend container instead of being blocked at the bottom of the table, users will find a little annoying the need to scroll down in order to scroll right. We already mentioned this in our conversations so nothing new, just wanted to write it down

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dej611 dej611 dej611 left review comments

@gvnmagni gvnmagni gvnmagni left review comments

@nickofthyme nickofthyme nickofthyme left review comments

@markov00 markov00 markov00 approved these changes

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@mbondyra @gvnmagni @dej611 @markov00 @nickofthyme