Skip to content

New Kibana app link order #66320

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
Merged
merged 5 commits into from
Jun 2, 2020
Merged

New Kibana app link order #66320

merged 5 commits into from
Jun 2, 2020

Conversation

@myasonik
Copy link
Contributor

@myasonik myasonik commented May 12, 2020 •
edited
Loading

Summary

Resolves #66279

Sets the order to:

  • Discover
  • Dashboard
  • Canvas
  • Maps
  • Machine Learning
  • Graph
  • Visualize

Also "forwardports" a fix for legacy links discovered while backporting a previous PR to 7.8. (Was only discovered because tests for this case are written in Discover, Dashboard, and Visualize and it's only broken if the app is a legacy app.)

Separate PR for 7.8 because of the massive amount of code change between versions because of Kibana App moving to NP.

@elasticmachine
Copy link
Contributor

elasticmachine commented May 12, 2020

Pinging @elastic/kibana-core-ui (Team:Core UI)

@myasonik myasonik requested a review from ryankeairns May 12, 2020 22:08
ryankeairns
ryankeairns approved these changes May 12, 2020
View reviewed changes
Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the quick turn around!

Screenshot 2020-05-12 17 33 46

@myasonik myasonik requested a review from a team May 13, 2020 17:59
flash1293
flash1293 approved these changes May 14, 2020
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Tested in firefox and works fine, LGTM

@rayafratkina
Copy link
Contributor

rayafratkina commented Jun 1, 2020

@ryankeairns @myasonik per discussion in #66279, this is no longer needed. However we do need to move Discover back to the top...

@myasonik myasonik requested a review from a team as a code owner June 2, 2020 16:33
@myasonik myasonik changed the title Revert Kibana app link order New Kibana app link order Jun 2, 2020
@myasonik myasonik removed the v7.8.0 label Jun 2, 2020
@myasonik myasonik mentioned this pull request Jun 2, 2020
Merged
@kibanamachine
Copy link
Contributor

kibanamachine commented Jun 2, 2020

💚 Build Succeeded

History

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

@myasonik myasonik requested a review from ryankeairns June 2, 2020 19:55
ryankeairns
ryankeairns approved these changes Jun 2, 2020
View reviewed changes
Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

The new order looks correct based upon most recent feedback - Discover first, Visualize last.

Screenshot 2020-06-02 16 44 35

@myasonik myasonik merged commit 0247df4 into elastic:master Jun 2, 2020
@myasonik myasonik deleted the fix/kibana-app-links-order branch June 2, 2020 21:46
@alexfrancoeur
Copy link

alexfrancoeur commented Jun 2, 2020

@myasonik @ryankeairns @kobelb I realize this is a bad day for backport requests at this point with #68000 and the telemetry issue, but we'd like to avoid randomly switching the order of links in the navigation. Would it be possible to ensure this change gets into 7.8?

@kobelb
Copy link
Contributor

kobelb commented Jun 2, 2020

Would it be possible to ensure this change gets into 7.8?

We're considering this one to be a bugfix, so I won't protest :)

@kobelb
Copy link
Contributor

kobelb commented Jun 2, 2020

@alexfrancoeur I believe this has been backported in another PR #67991

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 4, 2020
@kibanamachine
Copy link
Contributor

kibanamachine commented Jun 4, 2020

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 66320 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

kibanamachine commented Jun 5, 2020

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 66320 or prevent reminders by adding the backport:skip label.

1 similar comment
@kibanamachine
Copy link
Contributor

kibanamachine commented Jun 8, 2020

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 66320 or prevent reminders by adding the backport:skip label.

@myasonik myasonik added backported and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Jun 8, 2020
@marius-dr marius-dr mentioned this pull request Aug 3, 2020
Closed
@ryankeairns ryankeairns mentioned this pull request Nov 2, 2020
Closed
@myasonik myasonik mentioned this pull request Nov 2, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ryankeairns ryankeairns ryankeairns approved these changes

@flash1293 flash1293 flash1293 approved these changes

+1 more reviewer

@joshdover joshdover joshdover approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

backported REASSIGN from Team:Core UI Deprecated label for old Core UI team release_note:fix v7.9.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Navigation ordering in the Kibana section for 7.8

9 participants

@myasonik @elasticmachine @rayafratkina @kibanamachine @alexfrancoeur @kobelb @ryankeairns @flash1293 @joshdover