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

[Discover] Improve styling of graphs in sidebar #69440

Merged
merged 13 commits into from
Jun 26, 2020

Conversation

andreadelrio
Copy link
Contributor

Summary

This PR adjusts the styling of the graphs in the sidebar, making them more refined and similar to the styling in Lens.

graph_b-n

a_gif

Other changes:

  • We already had a tooltip showing the record count per value, I've added the value to the tooltip to account for the case when the value gets truncated due to limited space.

a_tooltip

Part of #68206

Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials

For maintainers

@andreadelrio andreadelrio requested a review from a team June 17, 2020 19:30
@andreadelrio andreadelrio requested a review from a team as a code owner June 17, 2020 19:30
@andreadelrio andreadelrio added Feature:Discover Discover Application Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Jun 17, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@andreadelrio andreadelrio requested a review from kertal June 17, 2020 19:31
@andreadelrio andreadelrio added release_note:skip Skip the PR/issue when compiling release notes v7.9.0 v8.0.0 labels Jun 17, 2020
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

I think these EuiProgress elements with the label and percentage shown is a great candidate for an EUI addition. We can still keep this PR but work in-tandem in EUI to establish a component/modify EuiProgress to create this pattern for reuse here and in Lens.


It would be great to also go ahead and replace the magnify icons with the regular plus/minus in circle icons.

@andreadelrio
Copy link
Contributor Author

Made changes so the Tooltip shows up when hovering the progress bar or the label/value. Also, the content now includes the percentage too.
tooltip

@andreadelrio
Copy link
Contributor Author

@elasticmachine merge upstream

@timroes
Copy link
Contributor

timroes commented Jun 23, 2020

@kertal Given the recent performance issues we were seeing in EuiTooltip, we should make sure this is properly evaluated and profiled compared to master before we're merging this in, that it doesn't cause us new performance regressions.

@kertal
Copy link
Member

kertal commented Jun 25, 2020

@elasticmachine merge upstream

@kertal kertal self-requested a review June 25, 2020 17:21
Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM, tested locally in Chrome, Firefox, Safari, works. it now uses less vertical screen space, less components, is prettier, and on top of that:
Bildschirmfoto 2020-06-25 um 19 22 08 👍

@kertal kertal requested a review from cchaos June 25, 2020 17:25
@andreadelrio
Copy link
Contributor Author

Made the changes. Updated behaviour:

changes

@andreadelrio andreadelrio requested a review from cchaos June 26, 2020 16:57
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

🎉 Awesome! Screenshots and code, LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
discover 80 +5 75

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@AlonaNadler
Copy link

@andreadelrio design looks good. QQ, why several of the available fields have a blue background and others, don't?

@andreadelrio
Copy link
Contributor Author

andreadelrio commented Jun 26, 2020

@andreadelrio design looks good. QQ, why several of the available fields have a blue background and others, don't?

@AlonaNadler Are you referring to the popular fields? This PR has only touched the graphs so all background behaviours are the same as the ones we currently have on master.

@andreadelrio andreadelrio merged commit 59925da into elastic:master Jun 26, 2020
andreadelrio added a commit to andreadelrio/kibana that referenced this pull request Jun 26, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 29, 2020
* master: (59 commits)
  [Lens] Fix broken test (elastic#70117)
  [SIEM] Import timeline fix (elastic#65448)
  [SECURITY SOLUTION][INGEST] UX update for ingest manager edit/create datasource for endpoint (elastic#70079)
  [Telemetry] Collector Schema (elastic#64942)
  [Endpoint] Add Endpoint empty states for onboarding (elastic#69626)
  Hide unused resolver buttons (elastic#70112)
  [Security] `Investigate in Resolver` Timeline Integration (elastic#70111)
  [Discover] Improve styling of graphs in sidebar (elastic#69440)
  [Metrics UI] Fix EuiTheme type issue (elastic#69735)
  skip failing suite (elastic#70104) (elastic#70103)
  [ENDPOINT] Hide the Timeline Flyout while on the Management Pages (elastic#69998)
  [SIEM][CASE] Persist callout when dismissed (elastic#68372)
  [SIEM][Exceptions] - Cleaned up and updated exception list item comment structure (elastic#69532)
  [Maps] remove indexing state from redux (elastic#69765)
  Add API integration test for deleting data streams. (elastic#70020)
  renames SIEM to Security Solution (elastic#70070)
  Adding saved_objects_page in OSS (elastic#69900)
  [Lens] Use accordion menus in field list for available and empty fields (elastic#68871)
  Dynamic uiActions & license support (elastic#68507)
  [SIEM] Update readme for timeline apis (elastic#67038)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 29, 2020
…bana into alerting/consumer-based-rbac

* 'alerting/consumer-based-rbac' of github.com:gmmorris/kibana: (25 commits)
  [Lens] Fix broken test (elastic#70117)
  [SIEM] Import timeline fix (elastic#65448)
  [SECURITY SOLUTION][INGEST] UX update for ingest manager edit/create datasource for endpoint (elastic#70079)
  [Telemetry] Collector Schema (elastic#64942)
  [Endpoint] Add Endpoint empty states for onboarding (elastic#69626)
  Hide unused resolver buttons (elastic#70112)
  [Security] `Investigate in Resolver` Timeline Integration (elastic#70111)
  [Discover] Improve styling of graphs in sidebar (elastic#69440)
  [Metrics UI] Fix EuiTheme type issue (elastic#69735)
  skip failing suite (elastic#70104) (elastic#70103)
  [ENDPOINT] Hide the Timeline Flyout while on the Management Pages (elastic#69998)
  [SIEM][CASE] Persist callout when dismissed (elastic#68372)
  [SIEM][Exceptions] - Cleaned up and updated exception list item comment structure (elastic#69532)
  [Maps] remove indexing state from redux (elastic#69765)
  Add API integration test for deleting data streams. (elastic#70020)
  renames SIEM to Security Solution (elastic#70070)
  Adding saved_objects_page in OSS (elastic#69900)
  [Lens] Use accordion menus in field list for available and empty fields (elastic#68871)
  Dynamic uiActions & license support (elastic#68507)
  [SIEM] Update readme for timeline apis (elastic#67038)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants