-
Notifications
You must be signed in to change notification settings - Fork 17
fix(AccessRights): no need in forms #2457
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
Conversation
|
bugbot run |
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.
Bug: Dialog Enter Key Submission Inconsistent
The RevokeAllRightsDialog does not support Enter key submission. This regression occurred because the form element's onSubmit handler was removed, and the Dialog component was not updated with onEnterKeyDown={onApply}. This creates an inconsistent user experience compared to other dialogs like ChangeOwnerDialog.
src/containers/Tenant/Diagnostics/AccessRights/components/RightsTable/RevokeAllRightsDialog.tsx#L110-L111
Lines 110 to 111 in 847c834
| return ( | |
| <Dialog open={open} size="s" onClose={onClose}> |
BugBot free trial expires on July 22, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
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.
Pull Request Overview
This PR addresses the removal of unnecessary form wrappers within the AccessRights-related dialogs and updates the naming of access rights constants for improved clarity and consistency. Key changes include:
- Renaming the "HumanReadableRights" constant to "RightsCodes" and applying the corresponding updates.
- Removing form wrappers from dialogs in RevokeAllRightsDialog and ChangeOwnerDialog, replacing form submission with direct event handlers.
- Updating the loading state logic in AccessRights.tsx to use the "isLoading" property from the query hook.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/containers/Tenant/GrantAccess/shared.ts | Renamed rights constant and updated its references. |
| src/containers/Tenant/Diagnostics/AccessRights/components/RightsTable/RevokeAllRightsDialog.tsx | Removed the form tag and added an onApply handler. |
| src/containers/Tenant/Diagnostics/AccessRights/components/ChangeOwnerDialog.tsx | Replaced form submission with onEnterKeyDown and onClickButtonApply handlers. |
| src/containers/Tenant/Diagnostics/AccessRights/AccessRights.tsx | Updated loading state usage from isFetching/currentData to isLoading. |
Comments suppressed due to low confidence (4)
src/containers/Tenant/GrantAccess/shared.ts:7
- [nitpick] The renaming to 'RightsCodes' improves clarity; please ensure that all related documentation and usage comments are updated accordingly.
export const RightsCodes: Record<string, number> = {
src/containers/Tenant/Diagnostics/AccessRights/components/RightsTable/RevokeAllRightsDialog.tsx:82
- [nitpick] Ensure that removing the form wrapper does not negatively impact keyboard-based submission and overall accessibility; verify that the Dialog component fully supports the onClickButtonApply mechanism for handling submissions.
const onApply = () => {
src/containers/Tenant/Diagnostics/AccessRights/components/ChangeOwnerDialog.tsx:85
- [nitpick] Verify that using the onEnterKeyDown prop on the Dialog component consistently handles form submission behavior and maintains accessibility, as it replaces the previous form onSubmit approach.
<Dialog open={open} size="s" onClose={onClose} onEnterKeyDown={onApply}>
src/containers/Tenant/Diagnostics/AccessRights/AccessRights.tsx:30
- [nitpick] Confirm that replacing the combined check of isFetching and currentData with isLoading accurately reflects the intended loading state without omitting necessary data handling logic.
);
CI Results
Test Status: ❌ FAILED
📊 Full Report
Test Changes Summary ⏭️1
⏭️ Skipped Tests (1)
Bundle Size: ✅
Current: 83.90 MB | Main: 83.90 MB
Diff: 1.38 KB (-0.00%)
✅ Bundle size unchanged.
ℹ️ CI Information