-
Notifications
You must be signed in to change notification settings - Fork 0
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
Hide dashboard overview and make dashboards management available #329
Hide dashboard overview and make dashboards management available #329
Conversation
Signed-off-by: yubonluo <[email protected]>
Signed-off-by: yubonluo <[email protected]>
…shboards into workspace-hide-dashboard-overview Signed-off-by: yubonluo <[email protected]> # Conflicts: # src/core/public/application/types.ts
Signed-off-by: yubonluo <[email protected]>
Signed-off-by: yubonluo <[email protected]>
return { | ||
status: AppStatus.inaccessible, | ||
navLinkStatus: AppNavLinkStatus.hidden, | ||
workspaceAccessibility: WorkspaceAccessibility.NO, |
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.
Change on registeration is enough I think. https://github.com/ruanyl/OpenSearch-Dashboards/pull/329/files#diff-62c80c4d71387ed50124856ec5921ca12c700df85c16ef77256b781582029585R118
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.
Have deleted the useless code.
!chromeless && | ||
workspaceAccessibility !== WorkspaceAccessibility.NO; | ||
if (category?.id === DEFAULT_APP_CATEGORIES.management.id) { | ||
return filterCondition && id === 'management'; |
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.
Could you please add some comment here to elaborate?
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.
The comment has been added
} else { | ||
return filterCondition; | ||
} |
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.
} else { | |
return filterCondition; | |
} | |
return filterCondition; |
Nit: better to remove useless else
.
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.
Have deleted else
Signed-off-by: yubonluo <[email protected]>
Signed-off-by: yubonluo <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## workspace-pr-integr #329 +/- ##
=======================================================
+ Coverage 35.17% 35.39% +0.22%
=======================================================
Files 1885 1890 +5
Lines 36421 36573 +152
Branches 6672 6708 +36
=======================================================
+ Hits 12810 12946 +136
- Misses 22761 22777 +16
Partials 850 850
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: yubonluo <[email protected]>
Signed-off-by: yubonluo <[email protected]>
@yubonluo do we still need to get this PR merged? There are some conflicts |
I will close this PR and submit the PR directly to OSD main. |
Description
When users create a new workspace, they can not choose dashboard overview and can choose dashboards management.
Issues Resolved
Screenshot
Testing the changes
Check List
yarn test:jest
yarn test:jest_integration