-
Notifications
You must be signed in to change notification settings - Fork 919
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] Hide first home breadcrumbs within workspace #7992
[Workspace] Hide first home breadcrumbs within workspace #7992
Conversation
@AMoo-Miki @zhongnansu can you help to review this change? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7992 +/- ##
=======================================
Coverage 60.52% 60.52%
=======================================
Files 3728 3728
Lines 88297 88300 +3
Branches 13662 13663 +1
=======================================
+ Hits 53439 53444 +5
+ Misses 31603 31602 -1
+ Partials 3255 3254 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
d48a366
to
d2d7262
Compare
Signed-off-by: Hailong Cui <ihailong@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>
d2d7262
to
b83d54f
Compare
Signed-off-by: Hailong Cui <ihailong@amazon.com>
@@ -73,6 +75,10 @@ export function HeaderBreadcrumbs({ | |||
crumbs = breadcrumbEnricher(crumbs); | |||
} | |||
|
|||
if (dropHomeFromBreadcrumb && crumbs.length && Object.hasOwn(crumbs[0], 'home')) { |
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.
Do we need to check if home
is true
?
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.
we could add this condition as well. There only have one place to set this additional property, i think it's ok only check existence.
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 we have the assumption that when home
is set, it must be true?
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.
Yes, i think so. home
is not a defined property on ChromeBreadcrumb
, ideally nobody else will set this property
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.
lgtm
|
* hide first home breadcrumbs within workspace Signed-off-by: Hailong Cui <ihailong@amazon.com> * Changeset file for PR #7992 created/updated * don't relay on text to drop home Signed-off-by: Hailong Cui <ihailong@amazon.com> * don't relay on text to drop home Signed-off-by: Hailong Cui <ihailong@amazon.com> --------- Signed-off-by: Hailong Cui <ihailong@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 63d0d32) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…ce (#8015) * [Workspace] Hide first home breadcrumbs within workspace (#7992) * hide first home breadcrumbs within workspace Signed-off-by: Hailong Cui <ihailong@amazon.com> * Changeset file for PR #7992 created/updated * don't relay on text to drop home Signed-off-by: Hailong Cui <ihailong@amazon.com> * don't relay on text to drop home Signed-off-by: Hailong Cui <ihailong@amazon.com> --------- Signed-off-by: Hailong Cui <ihailong@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 63d0d32) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix 2.x issue (#8021) Signed-off-by: Hailong Cui <ihailong@amazon.com> --------- Signed-off-by: Hailong Cui <ihailong@amazon.com> 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> Co-authored-by: Hailong Cui <ihailong@amazon.com>
…ce (#8015) * [Workspace] Hide first home breadcrumbs within workspace (#7992) * hide first home breadcrumbs within workspace Signed-off-by: Hailong Cui <ihailong@amazon.com> * Changeset file for PR #7992 created/updated * don't relay on text to drop home Signed-off-by: Hailong Cui <ihailong@amazon.com> * don't relay on text to drop home Signed-off-by: Hailong Cui <ihailong@amazon.com> --------- Signed-off-by: Hailong Cui <ihailong@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 63d0d32) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix 2.x issue (#8021) Signed-off-by: Hailong Cui <ihailong@amazon.com> --------- Signed-off-by: Hailong Cui <ihailong@amazon.com> 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> Co-authored-by: Hailong Cui <ihailong@amazon.com> (cherry picked from commit ee2656b) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…ce (#8015) (#8050) * [Workspace] Hide first home breadcrumbs within workspace (#7992) * hide first home breadcrumbs within workspace * Changeset file for PR #7992 created/updated * don't relay on text to drop home * don't relay on text to drop home --------- (cherry picked from commit 63d0d32) * fix 2.x issue (#8021) --------- (cherry picked from commit ee2656b) Signed-off-by: Hailong Cui <ihailong@amazon.com> 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> Co-authored-by: Hailong Cui <ihailong@amazon.com>
Description
Hide the home breadcrumbs when we are in a workspace, but keep it in recent popover
Issues Resolved
Screenshot
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration