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] 7.9 design cleanup #71444

Merged
merged 12 commits into from
Jul 14, 2020
Merged

[Lens] 7.9 design cleanup #71444

merged 12 commits into from
Jul 14, 2020

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Jul 13, 2020 •
edited
Loading

Addresses the toolbar and some form element settings (color picker)

ToolbarButton

I created a new component for reuse called <ToolbarButton> which lives solely in Lens, but at the top level (for now). This component extends a typical EuiButton, but with some added overrides to the text color version for adding the white background and bold text.

There is also a size="s" version that removes the shadow and keeps passes the size down to EuiButton for a shorter height.

Screen Shot 2020-07-13 at 09 40 58 AM

Dimension popover

I fixed the padding around the tabs in this popover and moved the padding to the content itself so the tabs would extend to the full width of the popover. Also, I updated the series color picker to use the correct placeholder and isClearable props added to the EUI component.

Screen Shot 2020-07-13 at 09 48 50 AM

Screen Shot 2020-07-13 at 09 48 35 AM

Screen Shot 2020-07-13 at 09 46 56 AM

Fill missing values

I moved the disabled help text to a tooltip wrapping the input instead of displaying it inline. This type of messaging is as important to be inline as help text for users to fill out the form or error messages and so a tooltip is less intrusive. This is also the same pattern we use for the series color picker.

Screen Shot 2020-07-13 at 09 53 19 AM

Checklist

Delete any items that are not applicable to this PR.

@cchaos cchaos added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Lens v7.9.0 labels Jul 13, 2020
@cchaos cchaos requested review from flash1293 and a team July 13, 2020 13:56
@cchaos cchaos requested a review from a team as a code owner July 13, 2020 13:56
@cchaos
Copy link
Contributor Author

cchaos commented Jul 13, 2020

@elasticmachine merge upstream

flash1293
flash1293 approved these changes Jul 13, 2020
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.

Tested in Firefox and left a small nit, but overall LGTM. I'm fine with having the toolbar button as a top level component in lens/public for now.

Side note: I noticed a small styling regression which is not caused by this PR in Firefox: There is no margin to the right and to the bottom of the screen - Chrome looks fine:
Screenshot 2020-07-13 at 17 50 38
Screenshot 2020-07-13 at 17 50 54

While you are at it fixing styling in Lens it would be great if you could take a quick look into that as well, but don't feel pressured. I can create a separate issue for this.

x-pack/plugins/lens/public/xy_visualization/xy_config_panel.tsx Show resolved Hide resolved
x-pack/plugins/lens/public/xy_visualization/xy_config_panel.tsx Show resolved Hide resolved
@cchaos
Copy link
Contributor Author

cchaos commented Jul 13, 2020

Odd, I'm not having that same rendering issue in Firefox in OSX. But I think it's dependent on how the OS handles scrollbars since that space is allotted for scrollbars and forces the "padding" by overflow: scroll. Since it's more OS dependent, and I can't test in anything other than OSX right now, I'd rather do a follow up for that.

Screen Shot 2020-07-13 at 12 56 52 PM

^^ FF in OSX

@flash1293
Copy link
Contributor

I’m also using OSX, it’s probably because you have a mouse connected. I will create a separate issue for this.

@cchaos
Copy link
Contributor Author

cchaos commented Jul 13, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
lens 142 +9 133

History

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

@cchaos cchaos merged commit ddd8fa8 into elastic:master Jul 14, 2020
@cchaos cchaos mentioned this pull request Jul 14, 2020
Merged
cchaos added a commit that referenced this pull request Jul 14, 2020
@cchaos
[Lens] 7.9 design cleanup (#71444) (#71573)
f74340f 
* Fix dimension popover layout and color picker “Auto”

* Created ToolbarButton

* Move disabled help text to tooltip for missing values

* Darker side panel backgrounds

* Adding to .asciidoc about where to put the SASS import

* Moving `SASS` guidelines to STYLEGUIDE.md

* Fix keyboard focus of XY settings popover

* Fix dark mode
@cchaos cchaos deleted the lens/7.9_design_cleanup branch October 1, 2020 02:02
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

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

Successfully merging this pull request may close these issues.
4 participants
@cchaos @flash1293 @kibanamachine @elasticmachine