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

Clean up dashboard padding and coloring #14811

Merged
merged 4 commits into from
Nov 8, 2017
Merged

Conversation

snide
Copy link
Contributor

@snide snide commented Nov 6, 2017

Cleans up the new dashboards with margins. Makes the following changes.

  • Tries to keep stylistic decisions in edit mode similar between the two types + two themes.
  • Fixes the resize handle in dark mode by adding a new encoded svg.
  • Tries somewhat to telegraph that only titles are draggable.
  • Fixes overall padding and spacing of dashboard items.
  • Switched edit icon to gear. The K6 font-awesome arrows don't read because they are too small.

Things that aren't super fixable without aggressive CSS.

  • Context menu arrow positioning. Better to have the gear aligned properly than the menu. The menu should later be fixed when the rest of the K7 options port over (specifically more than just bottom positioning).

@stacey-gammon I only tested this with some dummy dashboards I had. If you can run it through some of yours I'd appreciate it. I don't necessarily know where some of this stuff gets pulled in.

image

@snide snide requested a review from stacey-gammon November 6, 2017 22:22
@stacey-gammon
Copy link
Contributor

Looks great! To get tests passing you need to update the jest snapshots (run with - u. Are you synced with master? I notice some warnings on the console for jest tests that I thought should have been fixed.

@snide
Copy link
Contributor Author

snide commented Nov 7, 2017

@stacey-gammon Tests passing. This just merges to master right, does it need backporting?

@stacey-gammon
Copy link
Contributor

Failed on:

21:15:19          │ debg  findByCssSelector [href="#/visualize"]
21:15:19          │ debg  clickVisualizationByLinkText(Visualization TileMap)
21:15:29          │ debg  --- tryForTime failure: [POST http://localhost:9515/session/21ea5810e48093bcf0685b3775896faf/element / {"using":"partial link text","value":"Visualization TileMap"}] no such element: Unable to locate element: {"method":"partial link text","selector":"Visualization TileMap"}
21:15:29          │         (Session info: chrome=62.0.3202.75)
21:15:29          │         (Driver info: chromedriver=2.33.506092 (733a02544d189eeb751fe0d7ddca79a0ee28cce4),platform=Linux 4.4.0-45-generic x86_64)
21:15:40          │ debg  --- tryForTime failed again with the same message  ...
21:15:51          │ debg  --- tryForTime failed again with the same message  ...
21:16:01          │ debg  --- tryForTime failed again with the same message  ...
21:16:02          │ debg  Taking screenshot "/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/test/functional/screenshots/failure/visualize app tile map visualize app tile map chart should save with zoom level and load, take screenshot.png"
21:16:02        └- ✖ fail: "visualize app tile map visualize app tile map chart should save with zoom level and load, take screenshot"
21:16:02        │        tryForTime timeout: [POST http://localhost:9515/session/21ea5810e48093bcf0685b3775896faf/element / {"using":"partial link text","value":"Visualization TileMap"}] no such element: Unable to locate element: {"method":"partial link text","selector":"Visualization TileMap"}
21:16:02        │         (Session info: chrome=62.0.3202.75)
21:16:02        │         (Driver info: chromedriver=2.33.506092 (733a02544d189eeb751fe0d7ddca79a0ee28cce4),platform=Linux 4.4.0-45-generic x86_64)
21:16:02        │           at Server._post (/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/test/functional/services/remote/verbose_remote_logging.js:15:21)
21:16:02        │           at runRequest (/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/leadfoot/Session.js:92:40)

with screenshot:
visualize app tile map visualize app tile map chart should save with zoom level and load take screenshot 1

lets see if this is flaky,

jenkins, test this

@stacey-gammon
Copy link
Contributor

it will need to be backported to 6.x @snide. lets wait to merge till the ci passes.

Copy link
Contributor

@stacey-gammon stacey-gammon left a comment

Choose a reason for hiding this comment

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

LGTM

@snide snide merged commit 75006b7 into elastic:master Nov 8, 2017
stacey-gammon pushed a commit to stacey-gammon/kibana that referenced this pull request Nov 28, 2017
Fixes some coloring issues with dashboards with the new margin settings. Makes everything consistent between mode / theme.
stacey-gammon pushed a commit to stacey-gammon/kibana that referenced this pull request Nov 28, 2017
Fixes some coloring issues with dashboards with the new margin settings. Makes everything consistent between mode / theme.
stacey-gammon added a commit that referenced this pull request Nov 28, 2017
Fixes some coloring issues with dashboards with the new margin settings. Makes everything consistent between mode / theme.
stacey-gammon added a commit that referenced this pull request Nov 28, 2017
Fixes some coloring issues with dashboards with the new margin settings. Makes everything consistent between mode / theme.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dashboard Dashboard related features v6.1.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants