-
Notifications
You must be signed in to change notification settings - Fork 890
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
[navigation-next] Add new left navigation #7230
[navigation-next] Add new left navigation #7230
Conversation
…e_service.(opensearch-project#7093) Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Build and Verify on Linux (ciGroup4) failed because of #7057 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing changes, code looks good to me, since we already discussed the design quite a lot, I'm approving now!
return 50; | ||
} | ||
|
||
return 270; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: avoid magic number
navLinks: ChromeNavLink[]; | ||
suffix?: React.ReactElement; | ||
style?: React.CSSProperties; | ||
appId?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should optional props be at the end?
if (navLink.itemType === LinkItemType.CATEGORY) { | ||
return { | ||
id: navLink.category?.id ?? '', | ||
name: <div className="nav-link-item">{navLink.category?.label ?? ''}</div>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can category be empty label and id?
|
||
const width = useMemo(() => { | ||
if (!isNavOpen) { | ||
return 50; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of using magic number, can we give it a name, is it percentage or other unit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, will optimize this part.
import { Observable } from 'rxjs'; | ||
import { DEFAULT_NAV_GROUPS, NavGroupItemInMap } from '../../../../core/public'; | ||
|
||
export function SettingsIcon({ core }: { core: CoreStart }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this component covered by test?
The backport to
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-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-7230-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 38ae65b22c7f9d5bc7f18669489574d90a4ee9fe
# Push it to GitHub
git push --set-upstream origin backport/backport-7230-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x Then, create a pull request where the |
* [navigation-next] Add CollapsibleNavGroupEnabled component into chrome_service.(opensearch-project#7093) Signed-off-by: SuZhou-Joe <[email protected]> * feat: enable parent nav link id Signed-off-by: SuZhou-Joe <[email protected]> * feat: modify left navigation Signed-off-by: SuZhou-Joe <[email protected]> * feat: modify style Signed-off-by: SuZhou-Joe <[email protected]> * feat: enable left bottom Signed-off-by: SuZhou-Joe <[email protected]> * temp: merge Signed-off-by: SuZhou-Joe <[email protected]> * feat: save Signed-off-by: SuZhou-Joe <[email protected]> * feat: merge Signed-off-by: SuZhou-Joe <[email protected]> * temp change Signed-off-by: SuZhou-Joe <[email protected]> * temp change Signed-off-by: SuZhou-Joe <[email protected]> * fix: overview page can not be load Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless change Signed-off-by: SuZhou-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: update snapshot Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test error Signed-off-by: SuZhou-Joe <[email protected]> * fix: new application Signed-off-by: SuZhou-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhou-Joe <[email protected]> * feat: update category based on latest design Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: update snapshot Signed-off-by: SuZhou-Joe <[email protected]> * feat: do not emphasize see all link Signed-off-by: SuZhou-Joe <[email protected]> * Changeset file for PR opensearch-project#7230 created/updated * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: update 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>
* [navigation-next] Add CollapsibleNavGroupEnabled component into chrome_service.(#7093) * feat: enable parent nav link id * feat: modify left navigation * feat: modify style * feat: enable left bottom * temp: merge * feat: save * feat: merge * temp change * temp change * fix: overview page can not be load * feat: remove useless change * feat: add unit test * feat: update snapshot * fix: unit test error * fix: new application * feat: add unit test * feat: update category based on latest design * feat: update * feat: update snapshot * feat: do not emphasize see all link * Changeset file for PR #7230 created/updated * feat: update * feat: update --------- Signed-off-by: SuZhou-Joe <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
* [navigation-next] Add CollapsibleNavGroupEnabled component into chrome_service.(#7093) * feat: enable parent nav link id * feat: modify left navigation * feat: modify style * feat: enable left bottom * temp: merge * feat: save * feat: merge * temp change * temp change * fix: overview page can not be load * feat: remove useless change * feat: add unit test * feat: update snapshot * fix: unit test error * fix: new application * feat: add unit test * feat: update category based on latest design * feat: update * feat: update snapshot * feat: do not emphasize see all link * Changeset file for PR #7230 created/updated * feat: update * feat: update --------- 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 b28aa98) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* [navigation-next] Add CollapsibleNavGroupEnabled component into chrome_service.(#7093) * feat: enable parent nav link id * feat: modify left navigation * feat: modify style * feat: enable left bottom * temp: merge * feat: save * feat: merge * temp change * temp change * fix: overview page can not be load * feat: remove useless change * feat: add unit test * feat: update snapshot * fix: unit test error * fix: new application * feat: add unit test * feat: update category based on latest design * feat: update * feat: update snapshot * feat: do not emphasize see all link * Changeset file for PR #7230 created/updated * feat: update * feat: update --------- (cherry picked from commit b28aa98) 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>
Description
Issues Resolved
closes #7148
closes #7094
Screenshot
data source enabled / new navigation enabled
20240717154041829.mp4
data source enabled / new navigation disabled
20240717183614054.mp4
data source disabled / new navigation enabled
20240717155424312.mp4
data source disabled / new navigation disabled
20240717155203221.mp4
Testing the changes
new home page
config.Changelog
Check List
yarn test:jest
yarn test:jest_integration