-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Lens] fix empty state for pie #66206
[Lens] fix empty state for pie #66206
Conversation
Pinging @elastic/kibana-app (Team:KibanaApp) |
We already have custom chart type icons for Lens in the chart type switcher. I'd rather not add more glyphs as well for every chart type. I would have thought we'd just use a simple chart icon for every type of chart for all empty states. I'll consider what this should be. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested in Firefox in editor and on dashboard, works fine for me. Code LGTM, we could add a unit test for this behavior but no blocker for merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized you're trying to get this into 7.8. I think it's fine for now to ship with the pie chart icon for donut and treemaps. I'll continue to think on this state for 7.9.
The screenshots LGTM, though I didn't test locally. Thanks for getting it in so quickly!
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* upstream/master: (223 commits) [Ingest] Support root level yaml variables in agent stream template (elastic#66120) [Snapshot Restore] Fix error when deleting snapshots behind reverse proxy (elastic#66147) [Lens] fix empty state for pie (elastic#66206) [APM] Improve e2e tests (elastic#66373) [ML] Data Frame Analytics: Fix steps to be named phases. (elastic#65855) [Discover] Encode context link filter part (elastic#63831) [APM] Scope APM alert creation to environment (elastic#65681) Move Kibana Usage collectors outside the telemetry plugin (elastic#65663) [ML] Data Frame Analytics: Fix confusion matrix data grid width. (elastic#65818) Switch to core application service (elastic#63443) Removes use of prefer_v2_templates (elastic#66316) [Maps] handle case where fit to bounds does not match any documents (elastic#66307) log error instead of throw (elastic#66254) [plugin-helpers] remove outdated postinstall task (elastic#66324) Spaces - migrate default space and enter space view to KP (elastic#66098) [APM] Change plugin id for `apm_oss` to `apmOss` (elastic#66164) [Maps] convert map_selectors to TS (elastic#65905) [uptime/usage-collector] add missing await (elastic#66079) [Ingest] Add additional attributes to the Datasources Saved Object (elastic#66127) [Endpoint]EMT-339: add new policy response schema (elastic#66264) ...
Summary
Fixes #65692
The PR
visPie
to display the icon of pie chart no matter if it's pie, donut or treemap as this is the only one we have in EUI. @cchaos do you think it's worth to design the icons for donut and treemap?Checklist
Delete any items that are not applicable to this PR.
For maintainers