[ML] Reorganize ML navigation with top and sub level tabs #45220

alvarezmelissa87 · 2019-09-10T01:09:29Z

Summary

Reorganize navigation into 4 main areas: Anomaly Detection, Transforms, Data Frame Analytics, and Data Visualizer.

Each main area has sub-navigation pertaining to that area.
When clicking on a main area for the first time the default subsection selected will be the one to the far left.

Updated breadcrumbs to include main tab.

follow up coming for addingOverview main tab which will contain an overview of all jobs.

Checklist

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

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
~~- [ ] Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

~~- [ ] This was checked for breaking API changes and was labeled appropriately~~
~~- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately~~

elasticmachine · 2019-09-10T01:10:15Z

Pinging @elastic/ml-ui

elasticmachine · 2019-09-10T02:28:47Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-10T17:34:07Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

spalger · 2019-09-10T17:34:29Z

Please merge master

alvarezmelissa87 · 2019-09-10T18:39:55Z

retest

elasticmachine · 2019-09-10T20:07:41Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-10T23:53:31Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

walterra · 2019-09-11T07:52:48Z

About the breadcrumbs, in the screenshot there's:

Breadcrumb	Page
Machine Learning	Anomaly Detection Job Management page
Machine Learning > Anomaly Detection	Anomaly Explorer page
Machine Learning > Anomaly Detection > Settings	Anomaly Detection Settings page

I'm not sure about the start page but I think the Anomaly Explorer page should be Machine Learning > Anomaly Detection > Anomaly Explorer what do you think? For consistency I'm leaning towards having the start page also the full breadcrumb like Machine Learning > Anomaly Detection > Job Management esp. since we're getting the new overview page later.

jgowdyelastic · 2019-09-11T07:59:17Z

i agree with @walterra, we should be consistent. even if that now means the "root" of the main tabs will always have a page after it
e.g. Clicking on "Anomaly Detection" will auto select "Job Management", so the breadcrumbs will be "Machine Learning / Anomaly Detection / Job Management" by default.

x-pack/legacy/plugins/ml/public/datavisualizer/breadcrumbs.ts

x-pack/legacy/plugins/ml/public/datavisualizer/file_based/breadcrumbs.js

x-pack/legacy/plugins/ml/public/components/navigation_menu/main_tabs.tsx

x-pack/legacy/plugins/ml/public/breadcrumbs.js

walterra · 2019-09-11T12:26:06Z

@peteharverson It's definitely worth doing but I think we should do the analytics breadcrumbs in a follow up, they haven't been considered/implemented at all besides the main page for now.

jgowdyelastic · 2019-09-11T15:41:06Z

When a page doesn't include the time filter at the top right, the main tabs are lower.
The Data Visualizer page demonstrates this:

jgowdyelastic · 2019-09-11T16:05:43Z

I think the sub tabs look better condensed with a left margin to align them with the main tabs above.

vs current

peteharverson

A couple of extra tweaks to the Data Visualizer steps if possible, plus agree with @jgowdyelastic suggestions about the left margin for the sub menus, and the slight style difference on pages where there is no date picker. Otherwise looking good.

peteharverson · 2019-09-11T16:36:41Z

x-pack/legacy/plugins/ml/public/datavisualizer/file_based/breadcrumbs.js

+    DATA_VISUALIZER_BREADCRUMB,
+    {
+      text: i18n.translate('xpack.ml.dataVisualizer.fileBasedLabel', {
+        defaultMessage: 'File based'


Now I see it, I think maybe just File is better than File based!

peteharverson · 2019-09-11T16:37:31Z

x-pack/legacy/plugins/ml/public/datavisualizer/index_based/breadcrumbs.ts

+  ML_BREADCRUMB,
+  DATA_VISUALIZER_BREADCRUMB,
+  // @ts-ignore
+} from '../../breadcrumbs';

 export function getDataVisualizerBreadcrumbs() {


For the main page, is is possible for the breadcrumbs to be Machine Learning / Data Visualizer / Index here:

jgowdyelastic · 2019-09-11T16:32:30Z

x-pack/legacy/plugins/ml/public/components/navigation_menu/main_tabs.tsx

+  pathId?: string;
+}
+
+const TAB_DATA: { [key: string]: TabData } = {


this could be a Record
const TAB_DATA: Record<TabId, TabData> = {

jgowdyelastic · 2019-09-11T16:38:38Z

x-pack/legacy/plugins/ml/public/components/navigation_menu/main_tabs.tsx

+    <EuiTabs display="condensed">
+      {tabs.map((tab: Tab) => {
+        const id = tab.id;
+        const testSubject = TAB_DATA[id as TAB_DATA_PROPERTY].testSubject;


these type assertions aren't needed, id is a string.

jgowdyelastic · 2019-09-11T16:42:13Z

x-pack/legacy/plugins/ml/public/components/navigation_menu/navigation_menu.tsx

-];
+export type TabId = string;
+
+const tabSupport = {


giving this a type of:
type TabSupport = Record<string, string | null>;
means you don't need type assertion on lines 41 and 43.

jgowdyelastic · 2019-09-11T16:42:37Z

x-pack/legacy/plugins/ml/public/components/navigation_menu/navigation_menu.tsx

+  explorer: 'anomaly_detection',
+};
+
+type tabSupportId = keyof typeof tabSupport;


this isn't needed with the above suggested change

jgowdyelastic · 2019-09-11T16:50:42Z

x-pack/legacy/plugins/ml/public/components/navigation_menu/navigation_menu.tsx

 }

 export const NavigationMenu: FC<Props> = ({ tabId }) => {
  const disableLinks = isFullLicense() === false;
-  const showTabs = tabSupport.includes(tabId);
+  const showTabs = Object.keys(tabSupport).includes(tabId);


nit, could be const showTabs = typeof tabSupport[tabId] !== 'undefined';

jgowdyelastic · 2019-09-11T16:54:06Z

x-pack/legacy/plugins/ml/public/components/navigation_menu/tabs.tsx

-    },
-  ];
+function getTabs(tabId: TabId, disableLinks: boolean): Tab[] {
+  const TAB_MAP = {


could be const TAB_MAP: Record<TabId, Tab[]> = {

jgowdyelastic · 2019-09-11T16:54:20Z

x-pack/legacy/plugins/ml/public/components/navigation_menu/tabs.tsx

+
+  type TAB_MAP_ID = keyof typeof TAB_MAP;
+
+  return TAB_MAP[tabId as TAB_MAP_ID];


with the suggested change above this type assertion isn't needed

elasticmachine · 2019-09-11T16:59:53Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

peteharverson

Tested with latest edits, including on IE11, and LGTM

elasticmachine · 2019-09-11T18:42:09Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

alvarezmelissa87 · 2019-09-11T20:07:33Z

retest

elasticmachine · 2019-09-11T21:52:05Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

alvarezmelissa87 · 2019-09-12T00:17:09Z

retest

elasticmachine · 2019-09-12T01:53:27Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-12T03:26:34Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

jgowdyelastic

LGTM

) * Add main nav tabs with sub tabs for new nav * move transforms to top level main nav * Make top nav normal font weight * Update breadcrumbs to take top nav into account * proper spacing when settings selected * fix localization error * Fix functional tests. Update breadcrumbs * revert analytics breadcrumb update. save for follow up * ensure main/sub tabs align left * update dataVisualizer breadcrumbs * update typescript for tabs

…45517) * Add main nav tabs with sub tabs for new nav * move transforms to top level main nav * Make top nav normal font weight * Update breadcrumbs to take top nav into account * proper spacing when settings selected * fix localization error * Fix functional tests. Update breadcrumbs * revert analytics breadcrumb update. save for follow up * ensure main/sub tabs align left * update dataVisualizer breadcrumbs * update typescript for tabs

alvarezmelissa87 requested review from a team as code owners September 10, 2019 01:09

alvarezmelissa87 added :ml v7.5.0 labels Sep 10, 2019

alvarezmelissa87 added release_note:enhancement v8.0.0 labels Sep 10, 2019

alvarezmelissa87 force-pushed the ml-nav-reorg branch from 59844e5 to 796a910 Compare September 10, 2019 18:15

alvarezmelissa87 changed the title ~~WIP [ML] Reorganize ML navigation with top and sub level tabs~~ [ML] Reorganize ML navigation with top and sub level tabs Sep 10, 2019

alvarezmelissa87 self-assigned this Sep 10, 2019

alvarezmelissa87 requested review from jgowdyelastic and peteharverson September 10, 2019 18:46

peteharverson reviewed Sep 11, 2019

View reviewed changes

x-pack/legacy/plugins/ml/public/datavisualizer/breadcrumbs.ts Outdated Show resolved Hide resolved

x-pack/legacy/plugins/ml/public/datavisualizer/file_based/breadcrumbs.js Outdated Show resolved Hide resolved

peteharverson reviewed Sep 11, 2019

View reviewed changes

x-pack/legacy/plugins/ml/public/components/navigation_menu/main_tabs.tsx Show resolved Hide resolved

x-pack/legacy/plugins/ml/public/breadcrumbs.js Show resolved Hide resolved

peteharverson reviewed Sep 11, 2019

View reviewed changes

jgowdyelastic reviewed Sep 11, 2019

View reviewed changes

peteharverson approved these changes Sep 11, 2019

View reviewed changes

alvarezmelissa87 added 11 commits September 11, 2019 21:43

Add main nav tabs with sub tabs for new nav

e53804f

move transforms to top level main nav

8bcac48

Make top nav normal font weight

c8e6404

Update breadcrumbs to take top nav into account

5d67e23

proper spacing when settings selected

468f1dd

fix localization error

1c1982f

Fix functional tests. Update breadcrumbs

0fa6f49

revert analytics breadcrumb update. save for follow up

c46397a

ensure main/sub tabs align left

0372192

update dataVisualizer breadcrumbs

01f3610

update typescript for tabs

08f3cdf

alvarezmelissa87 force-pushed the ml-nav-reorg branch from e6635b2 to 08f3cdf Compare September 12, 2019 01:43

jgowdyelastic approved these changes Sep 12, 2019

View reviewed changes

alvarezmelissa87 merged commit 9e3a3dc into elastic:master Sep 12, 2019

alvarezmelissa87 mentioned this pull request Sep 12, 2019

[7.x] [ML] Reorganize ML navigation with top and sub level tabs (#45220) #45517

Merged

alvarezmelissa87 deleted the ml-nav-reorg branch September 12, 2019 14:20

This was referenced Sep 13, 2019

[ML] Transforms - access denied message not displayed #45612

Closed

[ML] Update navigation tests to handle main and sub tabs #45793

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Reorganize ML navigation with top and sub level tabs #45220

[ML] Reorganize ML navigation with top and sub level tabs #45220

alvarezmelissa87 commented Sep 10, 2019 •

edited

Loading

elasticmachine commented Sep 10, 2019

elasticmachine commented Sep 10, 2019

elasticmachine commented Sep 10, 2019

spalger commented Sep 10, 2019

alvarezmelissa87 commented Sep 10, 2019

elasticmachine commented Sep 10, 2019

elasticmachine commented Sep 10, 2019

walterra commented Sep 11, 2019

jgowdyelastic commented Sep 11, 2019 •

edited

Loading

walterra commented Sep 11, 2019

jgowdyelastic commented Sep 11, 2019 •

edited

Loading

jgowdyelastic commented Sep 11, 2019

peteharverson left a comment

peteharverson Sep 11, 2019

peteharverson Sep 11, 2019

jgowdyelastic Sep 11, 2019 •

edited

Loading

jgowdyelastic Sep 11, 2019

jgowdyelastic Sep 11, 2019

jgowdyelastic Sep 11, 2019

jgowdyelastic Sep 11, 2019

jgowdyelastic Sep 11, 2019

jgowdyelastic Sep 11, 2019

elasticmachine commented Sep 11, 2019

peteharverson left a comment

elasticmachine commented Sep 11, 2019

alvarezmelissa87 commented Sep 11, 2019

elasticmachine commented Sep 11, 2019

alvarezmelissa87 commented Sep 12, 2019

elasticmachine commented Sep 12, 2019

elasticmachine commented Sep 12, 2019

jgowdyelastic left a comment


		type TAB_MAP_ID = keyof typeof TAB_MAP;

		return TAB_MAP[tabId as TAB_MAP_ID];

[ML] Reorganize ML navigation with top and sub level tabs #45220

[ML] Reorganize ML navigation with top and sub level tabs #45220

Conversation

alvarezmelissa87 commented Sep 10, 2019 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Sep 10, 2019

elasticmachine commented Sep 10, 2019

💔 Build Failed

elasticmachine commented Sep 10, 2019

💔 Build Failed

spalger commented Sep 10, 2019

alvarezmelissa87 commented Sep 10, 2019

elasticmachine commented Sep 10, 2019

💔 Build Failed

elasticmachine commented Sep 10, 2019

💔 Build Failed

walterra commented Sep 11, 2019

jgowdyelastic commented Sep 11, 2019 • edited Loading

walterra commented Sep 11, 2019

jgowdyelastic commented Sep 11, 2019 • edited Loading

jgowdyelastic commented Sep 11, 2019

peteharverson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgowdyelastic Sep 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Sep 11, 2019

💔 Build Failed

peteharverson left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 11, 2019

💔 Build Failed

alvarezmelissa87 commented Sep 11, 2019

elasticmachine commented Sep 11, 2019

💔 Build Failed

alvarezmelissa87 commented Sep 12, 2019

elasticmachine commented Sep 12, 2019

💔 Build Failed

elasticmachine commented Sep 12, 2019

💚 Build Succeeded

jgowdyelastic left a comment

Choose a reason for hiding this comment

alvarezmelissa87 commented Sep 10, 2019 •

edited

Loading

jgowdyelastic commented Sep 11, 2019 •

edited

Loading

jgowdyelastic commented Sep 11, 2019 •

edited

Loading

jgowdyelastic Sep 11, 2019 •

edited

Loading