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

[ML] Add grouping to the side nav #123805

Merged
merged 28 commits into from
Jan 28, 2022
Merged

[ML] Add grouping to the side nav #123805

merged 28 commits into from
Jan 28, 2022

Conversation

darnautov
Copy link
Contributor

@darnautov darnautov commented Jan 26, 2022 •
edited
Loading

Summary

  • Adds groups to the side navigation
  • Adds new items to quickly open the Anomaly Explorer and the Single Metric Viewer pages. Both pages have been updated to request explicit job selection from the user using the job selection flyout.

image

Screen.Recording.2022-01-27.at.14.59.11.mov

Checklist

@darnautov darnautov self-assigned this Jan 26, 2022
@darnautov darnautov marked this pull request as ready for review January 27, 2022 13:55
@darnautov darnautov requested review from a team as code owners January 27, 2022 13:55
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@alvarezmelissa87
Copy link
Contributor

Tested and overall looks good but noticed I see this no jobs selected message for the explorer

image

but not for the single metric viewer when no job is selected. Is that intended?

image

alvarezmelissa87
alvarezmelissa87 approved these changes Jan 27, 2022
View reviewed changes
Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

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

LGTM except for the missing single metric viewer no job selected message.
Giving it a green so you're not blocked.

@mdefazio
Copy link
Contributor

@darnautov Is this PR just about adding grouping / updates to the side nav? Or does this PR also include the flyout selector flow?
Just trying to figure out where I should focus some feedback. Awesome to see all the progress in these views!

@darnautov
Copy link
Contributor Author

hey @mdefazio, thanks for taking a look! This PR is mostly about adding grouping and some updates to the side nav. Flyout selector has always been on both pages, I just added a logic to open it automatically when the user lands on the page with no job selected. Before the side nav changes the user could only go to the Anomaly Explorer and the Single Metric Viewer with predefined job selection (for instance from the Job list page or any other page that populates the URL with specific job/jobs).

@darnautov
Copy link
Contributor Author

darnautov commented Jan 28, 2022 •
edited
Loading

Tested and overall looks good but noticed I see this no jobs selected message for the explorer

image

but not for the single metric viewer when no job is selected. Is that intended?

image

Good spot, added in eea4506 cc @alvarezmelissa87

peteharverson
peteharverson approved these changes Jan 28, 2022
View reviewed changes
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested latest changes and LGTM

@darnautov darnautov enabled auto-merge (squash) January 28, 2022 15:09
auto-merge was automatically disabled January 28, 2022 16:03

Pull Request is not mergeable

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
ml 1611 1609 -2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 3.5MB 3.5MB +1.7KB
Unknown metric groups

miscellaneous assets size

id before after diff
ml 125.4KB 49.4KB -76.1KB

References to deprecated APIs

id before after diff
ml 73 71 -2

History

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

cc @darnautov

@darnautov darnautov merged commit d66aa7a into elastic:main Jan 28, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jan 28, 2022
@darnautov darnautov deleted the ml-side-nav-groups branch January 28, 2022 16:40
awahab07 pushed a commit to awahab07/kibana that referenced this pull request Jan 31, 2022
@darnautov @awahab07
[ML] Add grouping to the side nav (elastic#123805)
b9dc818 
* wip side nav grouping, open anomaly explorer

* single metric viewer job selection

* add top level data vis link

* resolve active route based on path

* rename hook

* fix breadcrumbs

* update Model Management structure

* fix i18n

* add test subj

* handle empty job selection on the Anomaly Explorer page

* checks for Anomaly Explorer page

* update SMV side nav link

* hide job selection control

* SMV job selection flyout on open

* fix spacer

* fix start trial panel positioning

* timeseriesOnly for SMV

* fix isSelected check for data-test-subj

* fix i18n

* model management experimental tag

* update model management navigation in functional tests

* update SMV test with unsupported job

* no jobs selected callout in smv

* show EuiLoadingContent while loading

* update breadcrumbs for model management pages

* update images
@peteharverson peteharverson added release_note:feature Makes this part of the condensed release notes and removed release_note:enhancement labels Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peteharverson peteharverson peteharverson approved these changes

@alvarezmelissa87 alvarezmelissa87 alvarezmelissa87 approved these changes

@walterra walterra Awaiting requested review from walterra

Assignees

@darnautov darnautov

Labels
backport:skip This commit does not require backporting :ml release_note:feature Makes this part of the condensed release notes Team:ML Team label for ML (also use :ml) v8.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@darnautov @elasticmachine @alvarezmelissa87 @mdefazio @kibana-ci @peteharverson @kibanamachine