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

[Data visualizer] Improve design of expanded rows #118125

Merged
merged 8 commits into from
Nov 15, 2021
Merged

[Data visualizer] Improve design of expanded rows #118125

merged 8 commits into from
Nov 15, 2021

Conversation

andreadelrio
Copy link
Contributor

@andreadelrio andreadelrio commented Nov 10, 2021 •
edited
Loading

Summary

Improves the design of the expanded rows with the following:

  • Removed border in panels in favor of using white space to separate panels, this prevents the case where there's a ton of white space inside a bordered panel
  • Added bottom border to tables
  • Adjusted alignment in table cells

Frame 6

  • Increased width of the summary table in date fields

Frame 7

Part of #117602

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space. Low High Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. High Low Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled. Medium High Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

@andreadelrio andreadelrio requested review from a team as code owners November 10, 2021 01:01
@elastic elastic deleted a comment from cla-checker-service bot Nov 10, 2021
@andreadelrio andreadelrio added release_note:skip Skip the PR/issue when compiling release notes v8.0.0 labels Nov 10, 2021
@qn895
Copy link
Member

qn895 commented Nov 10, 2021

Tested and the changes LGTM.

Small nit feedback: If we have the space to, personally I think if we remove the border we should add more spacing in between the sub-tables because currently it looks a bit crowded & clustered

Screen Shot 2021-11-10 at 11 48 26

The date panel can also feel really long because the distance between the word earliest and the actual date can get pretty far apart.

Screen Shot 2021-11-10 at 12 04 21

@ryankeairns
Copy link
Contributor

+1 for removing the borders. That greatly reduces the visual noise especially being we are already within a table which has dividers and density. Additionally, it de-emphasizes the varying whitespace within each 'box'.

Personally, the spacing in between doesn't feel too tight to me. I presume it is relative depending upon the screen/browser size - shrinking on smaller screens, for example.

Nice enhancements @andreadelrio

@qn895
Copy link
Member

qn895 commented Nov 11, 2021

Design changes LGTM 🎉

@andreadelrio
Copy link
Contributor Author

@elasticmachine merge upstream

@elastic elastic deleted a comment from kibanamachine Nov 15, 2021
@andreadelrio
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dataVisualizer 537.4KB 537.9KB +545.0B

History

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

@andreadelrio andreadelrio merged commit 80560c2 into elastic:main Nov 15, 2021
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 16, 2021
@jloleysens
Merge branch 'main' of github.com:elastic/kibana into reporting/poc-m…
0808a04 
…igrate-away-from-injected-css-js

* 'main' of github.com:elastic/kibana: (221 commits)
  [Reporting] Add log level to config (elastic#118319)
  [Metrics UI] Skip failing waffle chart color palette test (elastic#118527)
  [APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case (elastic#118328)
  skip flaky suites (elastic#99581, elastic#118356, elastic#118474)
  [Alerting] Initial implementation of alerting task `cancel()` (elastic#114289)
  chore(NA): creates pkg_npm_types bazel rule (elastic#116465)
  skip flaky suite (elastic#116892)
  Bump chromedriver to 95.0.0 (elastic#116724)
  [Data visualizer] Improve design of expanded rows (elastic#118125)
  [APM] prefer ECS field names for HTTP and URL (elastic#118485)
  Update query_debugging_in_development_and_production.md (elastic#118491)
  [Uptime] adjust Elastic Synthetics integration functional tests (elastic#118163)
  [kbn/rule-data-utils] add submodules and require public use them (elastic#117963)
  [DOCS] Refresh APM correlation screenshots (elastic#116723) (elastic#118577)
  Handles ns to ms conversion for event loop delay metrics (elastic#118447)
  [Cases] Rename functional tests folder (elastic#118490)
  dummy commit
  skip flaky suite (elastic#118593)
  Improve workpad schema validation (elastic#97838)
  skip flaky suite (elastic#118584)
  ...

# Conflicts:
#	src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.tsx
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 17, 2021
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 118125 or prevent reminders by adding the backport:skip label.

@andreadelrio andreadelrio mentioned this pull request Nov 17, 2021
Merged
andreadelrio added a commit to andreadelrio/kibana that referenced this pull request Nov 17, 2021
@andreadelrio
[Data visualizer] Improve design of expanded rows (elastic#118125)
b7edd97
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 18, 2021
andreadelrio added a commit that referenced this pull request Nov 18, 2021
@andreadelrio
[Data visualizer] Improve design of expanded rows (#118125) (#118970)
7f1bdc3
mbondyra pushed a commit to mbondyra/kibana that referenced this pull request Nov 19, 2021
@andreadelrio @mbondyra
[Data visualizer] Improve design of expanded rows (elastic#118125)
0c8ef7d
@qn895 qn895 added auto-backport Deprecated - use backport:version if exact versions are needed v8.1.0 labels Nov 19, 2021
@kibanamachine
Copy link
Contributor

💔 Backport failed

The backport operation could not be completed due to the following error:
There are no branches to backport to. Aborting.

The backport PRs will be merged automatically after passing CI.

To backport manually run:
node scripts/backport --pr 118125

@qn895 qn895 removed the auto-backport Deprecated - use backport:version if exact versions are needed label Nov 19, 2021
dmlemeshko pushed a commit that referenced this pull request Nov 29, 2021
@andreadelrio @dmlemeshko
[Data visualizer] Improve design of expanded rows (#118125)
b8d458a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qn895 qn895 qn895 approved these changes

@ryankeairns ryankeairns ryankeairns approved these changes

Assignees
No one assigned
Labels
release_note:skip Skip the PR/issue when compiling release notes v8.0.0 v8.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@andreadelrio @qn895 @ryankeairns @kibana-ci @kibanamachine