Project Details Page - Allow Edit and View Details #3286
Conversation
|
@jeff-phillips-18: GitHub didn't allow me to request PR reviews from the following users: jgiardino. Note that only opendatahub-io members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3286 +/- ##
==========================================
- Coverage 84.87% 84.84% -0.04%
==========================================
Files 1306 1308 +2
Lines 29196 29225 +29
Branches 7885 7893 +8
==========================================
+ Hits 24780 24795 +15
- Misses 4416 4430 +14
... and 76 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Updates look great @jeff-phillips-18 . Tested both self-provisioned and admin users, edit and view details working as expected. Left a comment below.
| isOpen={open} | ||
| popperProps={{ position: 'right', appendTo: getDashboardMainContainer() }} | ||
| > | ||
| <DropdownList> |
There was a problem hiding this comment.
Based on the mocks, should there be a divider between these "Edit" and "Delete" dropdown items?
There was a problem hiding this comment.
@jenny-s51 - We don't have design guidelines for action menus yet. When I look at other areas of the product, there are pages in the dashboard that consistently display a divider above Delete. But there are places where we have just Edit and Delete in a menu without a divider (Workbenches and Permissions list items in the Project details page).
The only guidelines I see on this topic within PF are under Menu > Red text menu:
A divider should be used to separate the destructive menu items from the non-destructive items.
We aren't using this variant on the Delete menu action (and maybe we should consider that when we draft guidelines), but the point about separating destructive menu items from the non-destructive menu items is the piece that I think is relevant here. I think this recommendation should apply regardless of how many menu items there are. So I am in favor of implementing the UI as designed in this case.
There was a problem hiding this comment.
@jenny-s51 @jgiardino Looking around the UI, I see:
- Connections (recently added) does not have the separator.
- Data connections does not have the separator
- Cluster storage does not have the separator
- Permissions (both user and group) do not have the separator
- Workbenches does not have the separator
- Pipeline servers does have the separator
- Pipeline versions does have the separator
I can add it here but it would be mostly inconsistent. I would suggest UX file an issue to make all of the menus consistent and we address all of them at once.
There was a problem hiding this comment.
@jeff-phillips-18 @jenny-s51 @jgiardino I agree with Jeff. I’d prefer to create a separate issue once we have the menu guidelines ready to update the styling for all menus at once and ensure consistency. For now, I’d vote to drop the divider from the implementation of the Project Details action.
There was a problem hiding this comment.
Thank you for your clarity here @jgiardino. Filing a separate ticket to make all of these menus consistent sounds like a good plan @jeff-phillips-18 @simrandhaliw.
| onClose={() => { | ||
| setDeleteOpen(false); | ||
| navigate('/projects'); | ||
| }} |
There was a problem hiding this comment.
Only navigate to the projects page if the project was actually deleted.
jeff-phillips-18
force-pushed
the
project-details-edits
branch
from
4ecc7be
to
9842608
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: christianvogt, jenny-s51 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Closes RHOAIENG-8571
Description
In the project details page, add a popover to show users the project's resource name and kind.
Also, add actions to allow a provisioned user to edit or delete the project.
How Has This Been Tested?
As a provisioned user, navigate to a project page.
Verify there is a popover indicator next to the project name.
Clicking the indicator shows a popover that shows the project's resource name and show
Projectfor the resource type.Click on the actions menu to the far right, verify both
Edit projectandDelete projectactions are shown.Verify that
Edit projectbring up theEdit projectmodal.Verify that
Delete projectbrings up theDelete projectmodal.As a non-provisioning user, navigate to a project page.
Verify there is a popover indicator next to the project name.
Clicking the indicator shows a popover that shows the project's resource name and show
Projectfor the resource type.Verify there is no actions menu shown to the far right.
Test Impact
Added cypress e2e tests.
Mocks
Ref: Figma prototype
Screen shots
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main/cc @jgiardino