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] Fixes vertical alignment validation messages #91878

Merged
merged 6 commits into from
Feb 25, 2021
Merged

[Lens] Fixes vertical alignment validation messages #91878

merged 6 commits into from
Feb 25, 2021

Conversation

dej611
Copy link
Contributor

@dej611 dej611 commented Feb 18, 2021

Summary

Fix #91845

This PR addresses a bug on the workspace layout after the recent EUI update ( #91210 ).

Checklist

@dej611 dej611 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.12.0 v7.13.0 labels Feb 18, 2021
@dej611 dej611 requested review from snide and a team February 18, 2021 16:39
@elasticmachine
Copy link
Contributor

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

flash1293
flash1293 approved these changes Feb 19, 2021
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There's lot of flex items and flex groups here, but I don't know whether doing custom css would be better to center things. It works fine for me so LGTM.

Not caused by this PR, but is there a reason we use a subdued color on dashboards vs. black in the editor? I'm sure we discussed this but I can't remember - it seemed a little strange to me but no big deal:
Screenshot 2021-02-19 at 17 56 54
Screenshot 2021-02-19 at 17 56 58

@snide probably knows a more elegant way to handle this.

@dej611
Copy link
Contributor Author

dej611 commented Feb 19, 2021

Dashboard uses a different layout than here. I think I tried that but it requires almost the same amount of components, so I left the original version with new wrappers.

Not caused by this PR, but is there a reason we use a subdued color on dashboards vs. black in the editor? I'm sure we discussed this but I can't remember - it seemed a little strange to me but no big deal:

Probably it should be subdued here as well.

@dej611
Copy link
Contributor Author

dej611 commented Feb 22, 2021

@elasticmachine merge upstream

@MichaelMarcialis
Copy link
Contributor

@dej611: I took a quick peek at this and I had some recommendations for reducing usage of the EuiFlexGroup component and introducing EuiEmptyPrompt for the error message. I've made a branch of this branch in my repo. Feel free to merge those changes into your branch: https://github.com/MichaelMarcialis/kibana/tree/fix/91845

@dej611
Copy link
Contributor Author

dej611 commented Feb 24, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 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
lens 918.2KB 918.0KB -159.0B

History

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

@dej611
Copy link
Contributor Author

dej611 commented Feb 24, 2021

@MichaelMarcialis merged your PR as it looked good to me 👍 .
Unless we want to change the text color to subdued in the main Lens editor there are not other planned changes to apply here AFAIK.

@dej611 dej611 merged commit 69272cb into elastic:master Feb 25, 2021
@dej611 dej611 deleted the fix/91845 branch February 25, 2021 08:42
This was referenced Feb 25, 2021
Merged
Merged
dej611 added a commit to dej611/kibana that referenced this pull request Feb 25, 2021
@dej611 @kibanamachine @MichaelMarcialis
[Lens] Fixes vertical alignment validation messages (elastic#91878)
63e4027 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Michael Marcialis <michael.marcialis@elastic.co>
dej611 added a commit to dej611/kibana that referenced this pull request Feb 25, 2021
@dej611 @kibanamachine @MichaelMarcialis
[Lens] Fixes vertical alignment validation messages (elastic#91878)
cb9ddee 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Michael Marcialis <michael.marcialis@elastic.co>
dej611 added a commit that referenced this pull request Feb 25, 2021
@dej611 @kibanamachine @MichaelMarcialis
[Lens] Fixes vertical alignment validation messages (#91878) (#92763)
716a228 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Michael Marcialis <michael.marcialis@elastic.co>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Michael Marcialis <michael.marcialis@elastic.co>
dej611 added a commit that referenced this pull request Feb 25, 2021
@dej611 @kibanamachine @MichaelMarcialis
[Lens] Fixes vertical alignment validation messages (#91878) (#92764)
0f8e88c 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Michael Marcialis <michael.marcialis@elastic.co>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Michael Marcialis <michael.marcialis@elastic.co>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 25, 2021
@gmmorris
Merge branch 'master' into task-manager/docs-monitoring
226deae 
* master: (38 commits)
  Fixes Cypress flake by adding pipe, click, and should (elastic#92762)
  [Discover] Fix filtering selected sidebar fields (elastic#91828)
  [ML] Fixes positions of calendar arrow buttons in start datafeed modal (elastic#92625)
  [dev/build_ts_refs] check that commit in outDirs matches mergeBase (elastic#92513)
  add dep on `@kbn/config` so it is built first
  [Expressions] [Lens] Add id and copyMetaFrom arg to mapColumn fn + add configurable onError argument to math fn (elastic#90481)
  [Lens] Fix Workspace hidden when using Safari (elastic#92616)
  [Lens] Fixes vertical alignment validation messages (elastic#91878)
  forbid x-elastic-product-origin header in elasticsearch configuration (elastic#92359)
  [Security Solution][Detections] Set default indicator path to reduce friction with new filebeat modules (elastic#92081)
  [ILM][Accessibility] Added A11y test for ILM new policy form. (elastic#92570)
  [Security Solution][Exceptions] - Fixes exceptions builder UI where invalid values can cause overwrites of other values (elastic#90634)
  Automatically generated Api documentation (elastic#86232)
  Increase index pattern select limit to 1000 (elastic#92093)
  [core.logging] Add RewriteAppender for filtering LogMeta. (elastic#91492)
  [Security Solution][Detection Rules] Update prebuilt rule threats to match schema (elastic#92281)
  [Security Solutions][Detection Engine] Fixes bug with not being able to duplicate indicator matches (elastic#92565)
  [Dashboard] Export appropriate references from byValue panels (elastic#91567)
  [Upgrade Assistant] Align code between branches (elastic#91862)
  [Security Solution][Case] Fix alerts push (elastic#91638)
  ...
@dej611 dej611 mentioned this pull request Mar 2, 2021
Merged
@dej611 dej611 mentioned this pull request Apr 8, 2021
Merged
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

@snide snide Awaiting requested review from snide

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.12.0 v7.13.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Lens] Validation errors are not vertically centered
5 participants
@dej611 @elasticmachine @MichaelMarcialis @kibanamachine @flash1293