[navigation]Refactor: flatten left nav in Analytics(all) use case #8332

SuZhou-Joe · 2024-09-25T10:14:34Z

Description

As the multi levels of left nav may lead to context missing and confusion, we will optimize the analytics(all) use case nav by flattening all the navigation. And in order to keep backward compatible, we will keep using showInAllNavGroup for those links within nav group category.

Dependencies

[navigation]feat: update category to flatten menus in analytics(all) use case dashboards-observability#2182
[navigation]feat: update category to flatten menus in analytics(all) use case anomaly-detection-dashboards-plugin#883
[navigation]feat: update category to flatten menus in analytics(all) use case dashboards-maps#674
[navigation]feat: update category to flatten menus in analytics(all) use case dashboards-reporting#448
[navigation]feat: update category to flatten menus in analytics(all) use case security-analytics-dashboards-plugin#1169
[navigation]feat: update category to flatten menus in analytics(all) use case alerting-dashboards-plugin#1114

Issues Resolved

Screenshot

Analytics(all)

Observability

Security analytics

Search

Essentials

Testing the changes

Changelog

feat: [navigation] flatten left nav in Analytics(all) use case
feat: [navigation] Adjust the appearances of the left navigation menu and the landing page

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

codecov · 2024-09-25T10:48:57Z

Codecov Report

Attention: Patch coverage is 80.76923% with 15 lines in your changes missing coverage. Please review.

Project coverage is 60.93%. Comparing base (774909e) to head (a10cb84).
Report is 135 commits behind head on main.

Files with missing lines	Patch %	Lines
.../core/public/chrome/nav_group/nav_group_service.ts	80.00%	1 Missing and 5 partials ⚠️
...public/chrome/ui/header/collapsible_nav_groups.tsx	55.55%	1 Missing and 3 partials ⚠️
...chrome/ui/header/collapsible_nav_group_enabled.tsx	60.00%	0 Missing and 2 partials ⚠️
.../chrome/ui/header/collapsible_nav_groups_label.tsx	86.66%	1 Missing and 1 partial ⚠️
.../public/components/feature_cards/feature_cards.tsx	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8332      +/-   ##
==========================================
- Coverage   60.93%   60.93%   -0.01%     
==========================================
  Files        3756     3759       +3     
  Lines       89225    89323      +98     
  Branches    13950    13970      +20     
==========================================
+ Hits        54373    54432      +59     
- Misses      31469    31494      +25     
- Partials     3383     3397      +14

Flag	Coverage Δ
Linux_1	`28.92% <1.42%> (+0.04%)`	⬆️
Linux_2	`56.30% <80.00%> (-0.05%)`	⬇️
Linux_3	`37.78% <1.42%> (+0.02%)`	⬆️
Linux_4	`29.94% <10.25%> (-0.03%)`	⬇️
Windows_1	`28.93% <1.42%> (+0.04%)`	⬆️
Windows_2	`56.25% <80.00%> (-0.05%)`	⬇️
Windows_3	`37.78% <1.42%> (+0.01%)`	⬆️
Windows_4	`29.94% <10.25%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: SuZhou-Joe <[email protected]>

src/core/public/chrome/nav_group/nav_group_service.ts

Signed-off-by: SuZhou-Joe <[email protected]>

src/core/public/chrome/nav_group/nav_group_service.ts

AMoo-Miki · 2024-10-02T04:14:32Z

src/core/public/chrome/ui/header/collapsible_nav_group_enabled.tsx

  Expanded = 270,
  Collapsed = 48, // The Collasped width is supposed to be aligned with the hamburger icon on the top left navigation.


For later: Not a fan of having layout decisions made in JS. We should plan to move these to CSS, as classes that we toggle, or as CSS or SASS variables.

src/core/public/chrome/ui/header/collapsible_nav_group_enabled_top.tsx

src/plugins/management/public/components/feature_cards/feature_cards.tsx

Signed-off-by: SuZhou-Joe <[email protected]>

ruanyl · 2024-10-02T18:55:47Z

src/core/public/chrome/nav_group/nav_group_service.ts

-            // It means we are in a workspace, we should only use the visible use cases
-            visibleUseCases.forEach((navGroup) => mapAppIdToNavGroup(navGroup));
-          }
+          // It means we are in a workspace, we should only use the visible use cases


Couldn't quite get this comment, could you elaborate a bit please?

Sure, updated.

ruanyl · 2024-10-02T18:59:03Z

src/core/public/chrome/nav_group/nav_group_service.ts

+
+    // Append all the links that do not have use case info to keep backward compatible
+    const linkIdsWithNavGroupInfo = Object.values(navGroupsMap).reduce((accumulator, navGroup) => {
+      if (!navGroup.type) {


Maybe a comment for the if condition here?

Sure, updated.

ruanyl · 2024-10-02T19:14:34Z

src/core/public/chrome/ui/header/collapsible_nav_group_enabled.scss

@@ -1,33 +1,57 @@
 .context-nav-wrapper {
  border: none !important;
+  border-top-right-radius: $euiSizeL;
+  border-bottom-right-radius: $euiSizeL;
+  background-color: $euiColorLightShade;


We should perhaps add `overflow: hidden` here, otherwise it looks like this

Thanks, fixed.

ruanyl · 2024-10-02T19:15:37Z

src/core/public/chrome/ui/header/collapsible_nav_group_enabled.scss

@@ -1,33 +1,57 @@
 .context-nav-wrapper {
  border: none !important;
+  border-top-right-radius: $euiSizeL;
+  border-bottom-right-radius: $euiSizeL;
+  background-color: $euiColorLightShade;


This background color doesn't look like the color in design, shall we use the color from design?

There is not a token for left nav in OUI yet, I'd like to merge this version first. We can raise another PR to adjust background color if we have any new updates.

It looks the background is one of the important change and affects the look & feel quite a bit, my comments below are also related, let me double check and ask UX to confirm.

If we'd like to merge this PR first, perhaps I'd rather prefer to use the color value from design first, then then replace it with the color defined in OUI color palettes when it's ready.

Or maybe we can have the background color change in a separate PR so that we can get the other changes in this PR merged asap.

@virajsanghvi has a PR to add the color to OUI https://github.com/opensearch-project/oui/pull/1430/files#diff-67cb04d06a675c67602ff65a876373033b16a759964be1b850bbfb4ca50bad6aR13

And confirmed with him that we can commit the changes and let him know so that he can update the color after the PR in OUI merged. @SuZhou-Joe

Sure, makes sense. I will use the color from design in this PR to unblock review.

ruanyl · 2024-10-02T19:30:51Z

src/plugins/workspace/public/components/workspace_selector/workspace_selector.tsx

The workspace selector button color doesn't look right, I guess on line 91, it should be,

<EuiPanel paddingSize="s" borderRadius="m" color="transparent" hasBorder>

Sure, updated.

ruanyl · 2024-10-02T19:35:16Z

src/core/public/chrome/ui/header/collapsible_nav_group_enabled.scss

-    padding: calc($euiSize / 4) $euiSize;
-    border-radius: $euiSize;
+    padding: $euiSizeS;
+    border-radius: $euiSizeS;


Couldn't see the current selected app been highlighted in left nav, the reason is the left nav background color is the same as highlight nav item background color

Sure, by using the new background color value, it should be able to tell the difference.

ruanyl · 2024-10-02T19:43:44Z

src/core/public/chrome/ui/header/collapsible_nav_groups.tsx

        /**
         * The href and onClick should both be undefined to make parent item rendered as accordion.
         */
        href: undefined,
        onClick: undefined,
+        'data-test-subj': undefined,


'data-test-subj': undefined by intention?

Yes, added some comments.

Signed-off-by: SuZhou-Joe <[email protected]>

Hailong-am · 2024-10-03T06:51:59Z

src/core/public/chrome/ui/header/variables.scss

@@ -0,0 +1 @@
+$ouiSideNavBackgroundColorTemp: lightOrDarkTheme(#ebe4df, #001c28); /* stylelint-disable-line */ 


should this go with theme change? when i switch to theme v7, the page looks like this

It is. a intermedia state, will change to a variable in OUI theme after https://github.com/opensearch-project/oui/pull/1430/files#diff-67cb04d06a675c67602ff65a876373033b16a759964be1b850bbfb4ca50bad6aR13 get merged.

) * feat: move the logic to construct navLinks in all use case to nav_group_service Signed-off-by: SuZhou-Joe <[email protected]> * feat: change category Signed-off-by: SuZhou-Joe <[email protected]> * feat: register search overview to all use case Signed-off-by: SuZhou-Joe <[email protected]> * Changeset file for PR #8332 created/updated * feat: fix bootstrap Signed-off-by: SuZhou-Joe <[email protected]> * feat: show icon in category to expand or collapse Signed-off-by: SuZhou-Joe <[email protected]> * feat: merge fix/fit-finish Signed-off-by: SuZhou-Joe <[email protected]> * feat: finish whole refactor Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: style update Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit tets Signed-off-by: SuZhou-Joe <[email protected]> * Changeset file for PR #8332 created/updated * feat: remove useless code Signed-off-by: SuZhou-Joe <[email protected]> * feat: update snapshot Signed-off-by: SuZhou-Joe <[email protected]> * fix: i18n Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: update style Signed-off-by: SuZhou-Joe <[email protected]> * feat: update style Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless scss Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: change icon color to text and update the style a little bit Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: update naming Signed-off-by: SuZhou-Joe <[email protected]> * feat: optimize code Signed-off-by: SuZhou-Joe <[email protected]> * feat: optimize code Signed-off-by: SuZhou-Joe <[email protected]> * feat: optimize code Signed-off-by: SuZhou-Joe <[email protected]> * fix: warning Signed-off-by: SuZhou-Joe <[email protected]> * fix: bootstrap Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: optimize code based on comments Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 6677891) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

) (#8466) * feat: move the logic to construct navLinks in all use case to nav_group_service * feat: change category * feat: register search overview to all use case * Changeset file for PR #8332 created/updated * feat: fix bootstrap * feat: show icon in category to expand or collapse * feat: merge fix/fit-finish * feat: finish whole refactor * feat: update * feat: update * feat: style update * fix: unit test * fix: unit tets * Changeset file for PR #8332 created/updated * feat: remove useless code * feat: update snapshot * fix: i18n * feat: update * feat: update * feat: update * feat: update style * feat: update style * feat: update * feat: update * feat: update * feat: remove useless scss * feat: update * feat: change icon color to text and update the style a little bit * feat: update * feat: update * feat: update naming * feat: optimize code * feat: optimize code * feat: optimize code * fix: warning * fix: bootstrap * feat: update * fix: unit test * feat: optimize code based on comments --------- (cherry picked from commit 6677891) Signed-off-by: SuZhou-Joe <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

github-actions bot added the distinguished-contributor label Sep 25, 2024

SuZhou-Joe added backport 2.x 2.17.1 and removed distinguished-contributor labels Sep 25, 2024

opensearch-changeset-bot bot added a commit to SuZhou-Joe/OpenSearch-Dashboards that referenced this pull request Sep 25, 2024

Changeset file for PR opensearch-project#8332 created/updated

e4e02a7

SuZhou-Joe added the navigation label Sep 25, 2024

SuZhou-Joe marked this pull request as ready for review September 29, 2024 07:33

SuZhou-Joe requested review from ananzh, kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, curq, bandinib-amzn, ruanyl and BionIT as code owners September 29, 2024 07:33

feat: update

89ba989

Signed-off-by: SuZhou-Joe <[email protected]>

AMoo-Miki reviewed Oct 2, 2024

View reviewed changes

src/core/public/chrome/nav_group/nav_group_service.ts Outdated Show resolved Hide resolved

SuZhou-Joe added 2 commits October 2, 2024 12:05

feat: update

ea25014

Signed-off-by: SuZhou-Joe <[email protected]>

feat: update naming

7da0bfe

Signed-off-by: SuZhou-Joe <[email protected]>

AMoo-Miki reviewed Oct 2, 2024

View reviewed changes

src/core/public/chrome/nav_group/nav_group_service.ts Outdated Show resolved Hide resolved

AMoo-Miki reviewed Oct 2, 2024

View reviewed changes

src/core/public/chrome/ui/header/collapsible_nav_group_enabled_top.tsx Outdated Show resolved Hide resolved

AMoo-Miki reviewed Oct 2, 2024

View reviewed changes

src/plugins/management/public/components/feature_cards/feature_cards.tsx Outdated Show resolved Hide resolved

SuZhou-Joe added 7 commits October 2, 2024 15:22

feat: optimize code

faded17

Signed-off-by: SuZhou-Joe <[email protected]>

feat: optimize code

bfc1f60

Signed-off-by: SuZhou-Joe <[email protected]>

feat: optimize code

fde8c9b

Signed-off-by: SuZhou-Joe <[email protected]>

fix: warning

9f7d497

Signed-off-by: SuZhou-Joe <[email protected]>

fix: bootstrap

0e083b1

Signed-off-by: SuZhou-Joe <[email protected]>

feat: update

da5e4fe

Signed-off-by: SuZhou-Joe <[email protected]>

fix: unit test

b6f4b16

Signed-off-by: SuZhou-Joe <[email protected]>

Hailong-am previously approved these changes Oct 2, 2024

View reviewed changes

SuZhou-Joe requested a review from ruanyl October 2, 2024 17:59

ruanyl reviewed Oct 2, 2024

View reviewed changes

ruanyl previously approved these changes Oct 3, 2024

View reviewed changes

feat: optimize code based on comments

a10cb84

Signed-off-by: SuZhou-Joe <[email protected]>

SuZhou-Joe dismissed stale reviews from ruanyl and Hailong-am via a10cb84 October 3, 2024 05:35

Hailong-am approved these changes Oct 3, 2024

View reviewed changes

ruanyl approved these changes Oct 3, 2024

View reviewed changes

Hailong-am reviewed Oct 3, 2024

View reviewed changes

SuZhou-Joe merged commit 6677891 into opensearch-project:main Oct 3, 2024
67 checks passed

opensearch-trigger-bot bot mentioned this pull request Oct 3, 2024

[Backport 2.x] [navigation]Refactor: flatten left nav in Analytics(all) use case #8466

Merged

SuZhou-Joe added the v2.18.0 label Oct 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[navigation]Refactor: flatten left nav in Analytics(all) use case #8332

[navigation]Refactor: flatten left nav in Analytics(all) use case #8332

SuZhou-Joe commented Sep 25, 2024 •

edited

Loading

codecov bot commented Sep 25, 2024 •

edited

Loading

AMoo-Miki Oct 2, 2024

ruanyl Oct 2, 2024

SuZhou-Joe Oct 3, 2024

ruanyl Oct 2, 2024

SuZhou-Joe Oct 3, 2024

ruanyl Oct 2, 2024

SuZhou-Joe Oct 3, 2024

ruanyl Oct 2, 2024

SuZhou-Joe Oct 3, 2024

ruanyl Oct 3, 2024

ruanyl Oct 3, 2024 •

edited

Loading

ruanyl Oct 3, 2024

SuZhou-Joe Oct 3, 2024

ruanyl Oct 2, 2024

SuZhou-Joe Oct 3, 2024

ruanyl Oct 2, 2024

SuZhou-Joe Oct 3, 2024

ruanyl Oct 2, 2024

SuZhou-Joe Oct 3, 2024

Hailong-am Oct 3, 2024

SuZhou-Joe Oct 3, 2024

		Expanded = 270,
		Collapsed = 48, // The Collasped width is supposed to be aligned with the hamburger icon on the top left navigation.

		@@ -0,0 +1 @@
		$ouiSideNavBackgroundColorTemp: lightOrDarkTheme(#ebe4df, #001c28); /* stylelint-disable-line */

[navigation]Refactor: flatten left nav in Analytics(all) use case #8332

[navigation]Refactor: flatten left nav in Analytics(all) use case #8332

Conversation

SuZhou-Joe commented Sep 25, 2024 • edited Loading

Description

Dependencies

Issues Resolved

Screenshot

Analytics(all)

Observability

Security analytics

Search

Essentials

Testing the changes

Changelog

Check List

codecov bot commented Sep 25, 2024 • edited Loading

Codecov Report

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruanyl Oct 3, 2024 • 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

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

SuZhou-Joe commented Sep 25, 2024 •

edited

Loading

codecov bot commented Sep 25, 2024 •

edited

Loading

ruanyl Oct 3, 2024 •

edited

Loading