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

New platform cleanup tabs #33425

Merged
merged 10 commits into from
Mar 20, 2019
Merged

New platform cleanup tabs #33425

merged 10 commits into from
Mar 20, 2019

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Mar 18, 2019 •
edited
Loading

Summary

This PR includes the removal of the angular-boostrap directives:

  • tabset
  • tab

To achieve that, tabs were replaced in 2 places in the code (screenshots attached) with a simple React + EUI implementation.

  • search profiler
  • timelion app help documentation

Checklist

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

For maintainers

@lizozom lizozom self-assigned this Mar 18, 2019
@lizozom
Copy link
Contributor Author

lizozom commented Mar 18, 2019

Search Profiler tabs replacement
Note special behavior when profiling a query that has aggregations and then another query that doesn't (tab switches back to the search tab).

image

@lizozom
Copy link
Contributor Author

lizozom commented Mar 18, 2019 •
edited
Loading

Timelion tabs replacement

To test this:

  • Timelion app must be re-enabled via the kibana.yrn file, by setting timelion.ui.enabled: true
  • Timelion tutorial must be enabled via Advanced Management > timelion:showTutorial

image

@lizozom lizozom added Feature:New Platform Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 v7.2.0 labels Mar 18, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@timroes timroes mentioned this pull request Mar 18, 2019
Closed
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@lizozom lizozom requested a review from cchaos March 18, 2019 18:10
@lizozom lizozom added the EUI label Mar 18, 2019
cchaos
cchaos reviewed Mar 18, 2019
View reviewed changes
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Just a couple nits. And by looking at your screenshot:

image

The aggregations tab says "Panel options" though the code says "Aggregation profile", so I just want to double-check that the code one is correct.

x-pack/plugins/searchprofiler/public/components/searchprofiler_tabs.js Outdated Show resolved Hide resolved
x-pack/plugins/searchprofiler/public/components/searchprofiler_tabs.js Outdated Show resolved Hide resolved
lukeelmers
lukeelmers requested changes Mar 19, 2019
View reviewed changes
Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

I haven’t gotten to test locally yet, but did a pass on the code and made a few comments (mostly nits and minor stuff).

Overall makes sense though! Glad to see more deleted angular lines ❤️

src/legacy/core_plugins/timelion/public/components/timelionhelp_tabs.js Outdated Show resolved Hide resolved
src/legacy/core_plugins/timelion/public/components/timelionhelp_tabs.js Outdated Show resolved Hide resolved
x-pack/plugins/searchprofiler/public/app.js Show resolved Hide resolved
x-pack/plugins/searchprofiler/public/components/searchprofiler_tabs.js Outdated Show resolved Hide resolved
x-pack/plugins/searchprofiler/public/components/searchprofiler_tabs.js Outdated Show resolved Hide resolved
x-pack/plugins/searchprofiler/public/components/searchprofiler_tabs.js Outdated Show resolved Hide resolved
x-pack/plugins/searchprofiler/public/components/searchprofiler_tabs_directive.js Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

lukeelmers
lukeelmers reviewed Mar 19, 2019
View reviewed changes
config/kibana.yml Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

lukeelmers
lukeelmers reviewed Mar 19, 2019
View reviewed changes
Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

Changes LGTM!

Tested the timelion tabs locally (Chrome OSX). But I was not able to get to the search profiler tabs. Is there anything special I need to do to get them to show up?

@lizozom
Copy link
Contributor Author

lizozom commented Mar 20, 2019

@lukeelmers you need to go to Dev Console > Switch to the Search Profiler tab > click on the Profile button

The default query will return only Query Options, and if you copy a query from any visualize chart, you'll also get the Aggrgation Options

lukeelmers
lukeelmers approved these changes Mar 20, 2019
View reviewed changes
Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

🤦‍♂️ of course. Thanks @lizozom! Tested and everything LGTM

@lizozom lizozom merged commit e9863f3 into elastic:master Mar 20, 2019
lizozom added a commit to lizozom/kibana that referenced this pull request Mar 20, 2019
@lizozom
New platform cleanup tabs (elastic#33425)
b5c3412 
This PR includes the removal of the angular-boostrap directives:

* tabset
* tab

To achieve that, tabs were replaced in 2 places in the code (screenshots attached) with a simple React + EUI implementation.

* search profiler
* timelion app help documentation
@lizozom lizozom mentioned this pull request Mar 20, 2019
Merged
lizozom added a commit that referenced this pull request Mar 20, 2019
@lizozom
New platform cleanup tabs (#33425) (#33597)
c331a8c 
This PR includes the removal of the angular-boostrap directives:

* tabset
* tab

To achieve that, tabs were replaced in 2 places in the code (screenshots attached) with a simple React + EUI implementation.

* search profiler
* timelion app help documentation
@timroes timroes added the chore label Mar 27, 2019
@lizozom lizozom deleted the new-platform-cleanup-tabs branch April 21, 2019 10:46
@cchaos cchaos added EUI and removed Feature:EUI labels Aug 18, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/eui-design (EUI)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cchaos cchaos cchaos left review comments

@lukeelmers lukeelmers lukeelmers approved these changes

Assignees

@lizozom lizozom

Labels
chore EUI Feature:New Platform Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.2.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lizozom @elasticmachine @cchaos @lukeelmers @timroes