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.

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

[Workspace][Feature] Left navigation menu adjustment #192

Merged
merged 14 commits into from
Oct 18, 2023
Merged

[Workspace][Feature] Left navigation menu adjustment #192

merged 14 commits into from
Oct 18, 2023

Conversation

yuye-aws
Copy link
Collaborator

@yuye-aws yuye-aws commented Sep 21, 2023

Description

The main changes of this PR are about left navigation menu:

  • When workspace feature is enabled, filter features by workspace.
  • Rank categorized and uncategorized features according to order field.
  • Refactor Recently visited as a feature category.
  • Updated corresponding tests and snapshots.

Issues Resolved

Screenshot

image

Testing the changes

Check List

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

@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2023

Codecov Report

Merging #192 (66d838c) into workspace-pr-integr (120d3b7) will increase coverage by 0.43%.
The diff coverage is 96.05%.

❗ Current head 66d838c differs from pull request most recent head 1e1e12a. Consider uploading reports for the commit 1e1e12a to get more accurate results

@@                   Coverage Diff                   @@
##           workspace-pr-integr     #192      +/-   ##
=======================================================
+ Coverage                66.11%   66.55%   +0.43%     
=======================================================
  Files                     3392     3419      +27     
  Lines                    65038    65439     +401     
  Branches                 10496    10507      +11     
=======================================================
+ Hits                     43000    43553     +553     
+ Misses                   19461    19271     -190     
- Partials                  2577     2615      +38     
Flag Coverage Δ
Linux_ ?
_1 35.00% <44.73%> (?)
_2 55.57% <100.00%> (+0.03%) ⬆️
_3 44.46% <9.30%> (?)
_4 34.85% <9.30%> (?)

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

Files Coverage Δ
src/core/public/chrome/chrome_service.mock.ts 100.00% <ø> (ø)
src/core/public/chrome/nav_links/nav_link.ts 66.66% <ø> (ø)
.../core/public/chrome/nav_links/nav_links_service.ts 100.00% <100.00%> (ø)
...c/core/public/chrome/ui/header/collapsible_nav.tsx 98.03% <100.00%> (+2.58%) ⬆️
src/core/public/chrome/ui/header/nav_link.tsx 69.56% <100.00%> (+9.56%) ⬆️
...c/core/public/workspace/workspaces_service.mock.ts 100.00% <100.00%> (ø)
src/core/public/workspace/workspaces_service.ts 88.88% <ø> (ø)
src/core/utils/default_app_categories.ts 100.00% <ø> (ø)
...ment_section/objects_table/saved_objects_table.tsx 67.98% <ø> (ø)
src/plugins/workspace/public/utils.ts 100.00% <100.00%> (ø)
... and 1 more

... and 93 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ruanyl
Copy link
Owner

ruanyl commented Sep 22, 2023

@yuye-aws A lot tests failed, please take a look. And please also add PR descriptions

@yuye-aws yuye-aws changed the title feat: left navigation menu [Workspace][Feature] Left navigation menu adjustment Sep 22, 2023
src/core/public/chrome/nav_links/nav_links_service.ts Outdated Show resolved Hide resolved
src/core/public/chrome/nav_links/nav_links_service.ts Outdated Show resolved Hide resolved
src/core/public/chrome/ui/header/collapsible_nav.tsx Outdated Show resolved Hide resolved
src/core/public/index.ts Show resolved Hide resolved
src/plugins/workspace/public/plugin.ts Show resolved Hide resolved
);
},

setNavLinks: (filteredNavLinks: ReadonlyMap<string, ChromeNavLink>) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
setNavLinks: (filteredNavLinks: ReadonlyMap<string, ChromeNavLink>) => {
setNavLinks: (navLinks: ReadonlyMap<string, ChromeNavLink>) => {

@@ -132,36 +145,49 @@ export class NavLinksService {
// manual link modifications to be able to re-apply then after every
// availableApps$ changes.
const linkUpdaters$ = new BehaviorSubject<LinksUpdater[]>([]);
const navLinks$ = new BehaviorSubject<ReadonlyMap<string, NavLinkWrapper>>(new Map());
const allNavLinks$ = new BehaviorSubject<ReadonlyMap<string, NavLinkWrapper>>(new Map());
Copy link
Owner

Choose a reason for hiding this comment

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

We can move navLinks$ here so it align with existing pattern.

    const navLinks$ = new BehaviorSubject<ReadonlyMap<string, ChromeNavLink> | undefined>(
    undefined
  );

});

const forceAppSwitcherNavigation$ = new BehaviorSubject(false);

return {
getNavLinks$: () => {
return navLinks$.pipe(map(sortNavLinks), takeUntil(this.stop$));
return combineLatest([allNavLinks$, this.navLinks$]).pipe(
Copy link
Owner

Choose a reason for hiding this comment

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

Is this.navLinks$ the filtered links which will be displayed? If so, can we give it a precise name for readability? Something like displayedLinks?

Comment on lines 245 to 250
function sortChromeNavLinks(chromeNavLinks: ReadonlyMap<string, ChromeNavLink>) {
return sortBy(
[...chromeNavLinks.values()].map((link) => link as Readonly<ChromeNavLink>),
'order'
);
}
Copy link
Owner

Choose a reason for hiding this comment

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

I may combine sortNavLinks and sortChromeNavLinks like this:

function sortLinks(navLinks: ReadonlyMap<string, NavLinkWrapper | ChromeNavLink>) {
  return sortBy(
    [...navLinks.values()].map((link) => 'properties' in link ? link.properties : link),
    'order'
  );
}

ruanyl and others added 14 commits October 17, 2023 15:02
Signed-off-by: Yulong Ruan <ruanyl@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
@yuye-aws yuye-aws requested a review from ruanyl October 17, 2023 09:03
@yuye-aws yuye-aws merged commit c271e74 into ruanyl:workspace-pr-integr Oct 18, 2023
32 checks passed
@opensearch-workspace-development

The backport to workspace failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-workspace workspace
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-workspace
# Create a new branch
git switch --create backport/backport-192-to-workspace
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c271e741f8bb2aee06aeff1976249cb37813f3cd
# Push it to GitHub
git push --set-upstream origin backport/backport-192-to-workspace
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-workspace

Then, create a pull request where the base branch is workspace and the compare/head branch is backport/backport-192-to-workspace.

yuye-aws added a commit that referenced this pull request Oct 26, 2023
* [Workspace][Feature] Left navigation menu adjustment (#192)

* add util function to filter workspace feature by wildcard

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>

* resolve conflict

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* update tests and snapshots

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* small adjustment to left menu

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* resolve git conflict

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* rename nav link service function

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* unit test for workspace plugin.ts

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* update snapshots

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

---------

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Co-authored-by: Yulong Ruan <ruanyl@amazon.com>

* resolve conflict

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* update snapshots

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* resolve conflicts

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* restore snapshot

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* update tests

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

---------

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Co-authored-by: Yulong Ruan <ruanyl@amazon.com>
wanglam pushed a commit that referenced this pull request Feb 26, 2024
* add util function to filter workspace feature by wildcard

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>

* resolve conflict

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* update tests and snapshots

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* small adjustment to left menu

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* resolve git conflict

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* rename nav link service function

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* unit test for workspace plugin.ts

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* update snapshots

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

---------

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Co-authored-by: Yulong Ruan <ruanyl@amazon.com>
SuZhou-Joe pushed a commit that referenced this pull request Mar 18, 2024
* add util function to filter workspace feature by wildcard

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>

* resolve conflict

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* update tests and snapshots

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* small adjustment to left menu

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* resolve git conflict

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* rename nav link service function

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* unit test for workspace plugin.ts

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* update snapshots

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

---------

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Co-authored-by: Yulong Ruan <ruanyl@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants