Skip to content

[Search Profiler] Quick style fix up including dark theme - #33445

Merged
cchaos merged 7 commits into
elastic:masterfrom
cchaos:search-profiler-design-fix-up
Apr 2, 2019
Merged

[Search Profiler] Quick style fix up including dark theme#33445
cchaos merged 7 commits into
elastic:masterfrom
cchaos:search-profiler-design-fix-up

Conversation

@cchaos

@cchaos cchaos commented Mar 18, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

I noticed that the search profiler hadn't been updated in a while and was especially broken in dark mode. This fixes that up. Here's a quick gif of it all together in dark mode:

The only major change is that the details view has turned into a "pseudo"-flyout (since it's not React, this is just using the .euiFlyout classes).

It would be great to get this in for 7.0 so that it's not broken for those trying to use it in dark mode.

Fixes #21741, #18509, #18426, #18355, #18041

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@cchaos cchaos added Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. :profiler Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// release_note:skip Skip the PR/issue when compiling release notes v7.2.0 labels Mar 18, 2019
@cchaos
cchaos requested a review from a team as a code owner March 18, 2019 20:07
@elasticmachine

elasticmachine commented Mar 18, 2019

Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-design

@elasticmachine

elasticmachine commented Mar 18, 2019

Copy link
Copy Markdown
Contributor

Pinging @elastic/es-ui

@elasticmachine

elasticmachine commented Mar 18, 2019

Copy link
Copy Markdown
Contributor

💔 Build Failed

@cchaos
cchaos force-pushed the search-profiler-design-fix-up branch from 8634f90 to 9a76564 Compare March 18, 2019 21:20
@elasticmachine

elasticmachine commented Mar 18, 2019

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@cchaos
cchaos force-pushed the search-profiler-design-fix-up branch from 9a76564 to 51f86b7 Compare March 19, 2019 20:33
@cchaos
cchaos requested a review from bmcconaghy March 19, 2019 20:33
@cchaos

cchaos commented Mar 19, 2019

Copy link
Copy Markdown
Contributor Author

I've updated based on your comment @bmcconaghy

@elasticmachine

elasticmachine commented Mar 19, 2019

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@bmcconaghy

bmcconaghy commented Mar 20, 2019

Copy link
Copy Markdown
Contributor

@cchaos looking great. the only thing I noticed was the flyout header looks a little cramped:
image

@cchaos cchaos added v7.0.1 and removed v7.0 labels Mar 21, 2019
@cchaos cchaos mentioned this pull request Mar 21, 2019
Merged
@cchaos

cchaos commented Mar 21, 2019

Copy link
Copy Markdown
Contributor Author

I had to start a custom 7.0 backport here #33664

Because the tabset directive was removed only down to 7.x which I will be updating shortly

@cchaos
cchaos force-pushed the search-profiler-design-fix-up branch from b375456 to df464b6 Compare March 21, 2019 16:55
@cchaos

cchaos commented Mar 21, 2019

Copy link
Copy Markdown
Contributor Author

@bmcconaghy Flyout title is fixed up and same with the new tabset.

@elasticmachine

elasticmachine commented Mar 21, 2019

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@cchaos

cchaos commented Mar 22, 2019

Copy link
Copy Markdown
Contributor Author

@bmcconaghy This is ready for final review or 👍

@bmcconaghy

bmcconaghy commented Apr 2, 2019

Copy link
Copy Markdown
Contributor

Sorry for the delay, was on PTO until today.

bmcconaghy
bmcconaghy approved these changes Apr 2, 2019
View reviewed changes

@bmcconaghy bmcconaghy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks for the major improvement!

@cchaos
cchaos force-pushed the search-profiler-design-fix-up branch from df464b6 to 5cec956 Compare April 2, 2019 16:30
snide
snide approved these changes Apr 2, 2019
View reviewed changes

@snide snide left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I did a very quick code only scan of this. The CSS looks ok to me. Using the EUI css directly will likely cause us some trouble one day, but the components used are pretty stable and I don't see a quick solution other than converting this to React (which hopefully gets done by engineering later).

@snide

snide commented Apr 2, 2019

Copy link
Copy Markdown
Contributor

@cchaos any reason you have this with a skip label for release notes? seems like an enhancement even if it's just visual?

@cchaos cchaos added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Apr 2, 2019
@cchaos

cchaos commented Apr 2, 2019

Copy link
Copy Markdown
Contributor Author

Because I'm confused by the release_note labels... Changed it

@elasticmachine

elasticmachine commented Apr 2, 2019

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@cchaos
cchaos merged commit e4d3b5d into elastic:master Apr 2, 2019
@cchaos cchaos mentioned this pull request Apr 2, 2019
Merged
cchaos added a commit to cchaos/kibana that referenced this pull request Apr 2, 2019
@cchaos
[Search Profiler] Quick style fix up including dark theme (elastic#33445
3c98319 
)
@cchaos

cchaos commented Apr 2, 2019

Copy link
Copy Markdown
Contributor Author

[7.0] #33664
[7.x] #34371

@cchaos
cchaos deleted the search-profiler-design-fix-up branch April 2, 2019 17:29
cchaos added a commit that referenced this pull request Apr 2, 2019
@cchaos
[Search Profiler] Quick style fix up including dark theme (#33445) (#…
7185fdf 
…34371)
This was referenced May 1, 2019
Closed
Closed
Closed
Closed
Closed
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
@cchaos
[Search Profiler] Quick style fix up including dark theme (elastic#33445
276030c 
)

* Quick unrelated fixes

* [Search Profiler] Quick fix style fix up including dark theme

* Fixed IE

* Remove unused translation

* Make the main panel always visible and scroll independently

* Space out flyout title

* Fix up for the new tabs directive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@snide snide snide approved these changes
@bmcconaghy bmcconaghy bmcconaghy approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release_note:enhancement Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. v7.0.1 v7.2.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Multiple UI problems in Search Profiler

5 participants

@cchaos @elasticmachine @bmcconaghy @snide @cjcenizal