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

[Lens] Adjust Lens Visualization Padding in Dashboards #81911

Merged
merged 8 commits into from
Oct 29, 2020
Merged

[Lens] Adjust Lens Visualization Padding in Dashboards #81911

merged 8 commits into from
Oct 29, 2020

Conversation

MichaelMarcialis
Copy link
Contributor

@MichaelMarcialis MichaelMarcialis commented Oct 28, 2020 •
edited
Loading

Summary

The primary goal of this PR is to reduce the container padding for Lens visualizations in Dashboards. Secondarily, some small changes were made to better match the different visualization types in Dashboard, including Lens, TSVB and classic/older visualizations. Notable changes include:

  • Reduce Lens visualization container vertical/horizontal padding from 16px to 8px in Dashboard.
  • Reduce classic/older visualization vertical overflow offset padding from 10px to 8px.
  • Change classic/older visualization axes title color to match Lens and TSVB.
  • Remove TSVB faux padding border styles to match Lens padding.

Originating Issue: elastic/elastic-charts#736

image

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@MichaelMarcialis MichaelMarcialis added Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Lens v7.11 labels Oct 28, 2020
@AlonaNadler
Copy link

thanks @MichaelMarcialis
Can we do anything with the right side emptiness?

Also the change is good but there is room for more. @markov00 which team should help with this?

@MichaelMarcialis MichaelMarcialis marked this pull request as ready for review October 28, 2020 16:34
@MichaelMarcialis MichaelMarcialis requested a review from a team October 28, 2020 16:34
@MichaelMarcialis MichaelMarcialis requested a review from a team as a code owner October 28, 2020 16:34
@elasticmachine
Copy link
Contributor

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

@MichaelMarcialis MichaelMarcialis changed the title Dashboard lens viz padding [Lens] Adjust Lens Visualization Padding in Dashboards Oct 28, 2020
@MichaelMarcialis
Copy link
Contributor Author

Can we do anything with the right side emptiness?

Hey, @AlonaNadler. Good question. I think fixing the right-side whitespace will require a change to Elastic charts, as it relates to the issue that @nickofthyme mentions here: elastic/elastic-charts#736 (comment). This PR is aimed at tightening and tidying up the areas that we can outside of Elastic charts.

@MichaelMarcialis
Revert "update snapshots"
d4be161 
This reverts commit c63cf2b.
@MichaelMarcialis MichaelMarcialis removed the release_note:skip Skip the PR/issue when compiling release notes label Oct 28, 2020
@nickofthyme
Copy link
Contributor

@MichaelMarcialis unfortunately no because of the "nice" ticks option is not yet a thing. elastic/elastic-charts#397

@MichaelMarcialis MichaelMarcialis added the release_note:skip Skip the PR/issue when compiling release notes label Oct 28, 2020
flash1293
flash1293 approved these changes Oct 29, 2020
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Tested in Chrome, Lens change LGTM

@flash1293
Copy link
Contributor

flash1293 commented Oct 29, 2020 •
edited
Loading

Seems like to get the build green you need to update a screenshot test baseline. Let me know if I can help with this.

@timroes timroes added v7.11.0 and removed v7.11 labels Oct 29, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

async chunks size

id before after diff
visTypeTimeseries 1.7MB 1.7MB -153.0B

page load bundle size

id before after diff
visTypeTimeseries 133.0KB 132.7KB -304.0B
visTypeVislib 207.9KB 207.9KB -4.0B
total -308.0B

History

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

@MichaelMarcialis MichaelMarcialis merged commit 2a38461 into elastic:master Oct 29, 2020
@MichaelMarcialis MichaelMarcialis deleted the dashboard-lens-viz-padding branch October 29, 2020 20:52
@MichaelMarcialis MichaelMarcialis mentioned this pull request Oct 29, 2020
Merged
MichaelMarcialis added a commit that referenced this pull request Oct 29, 2020
@MichaelMarcialis
[Lens] Adjust Lens Visualization Padding in Dashboards (#81911) (#82079)
195b1cf 
* reduce padding on lens visualizations in dashboard

* tweak padding and axes title colors to match lens

* remove faux padding (border) to match lens padding

* update snapshots

* Revert "update snapshots"

This reverts commit c63cf2b.

* update functional test baseline screenshot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flash1293 flash1293 flash1293 approved these changes

@ryankeairns ryankeairns ryankeairns approved these changes

@cchaos cchaos Awaiting requested review from cchaos

@markov00 markov00 Awaiting requested review from markov00

@AlonaNadler AlonaNadler Awaiting requested review from AlonaNadler

Assignees
No one assigned
Labels
Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@MichaelMarcialis @AlonaNadler @elasticmachine @nickofthyme @flash1293 @kibanamachine @ryankeairns @timroes