-
Notifications
You must be signed in to change notification settings - Fork 89
Migrate properties, domain verification, and messaging templates tables to Ant Design #7196
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
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.
Greptile Overview
Greptile Summary
This PR successfully migrates three table views (properties, domain verification, and messaging templates) from Chakra UI to Ant Design, resulting in a net reduction of ~180 lines of code.
- Extracted properties table logic into a new
usePropertiesTablehook following the project's pattern for custom table hooks - Properly uses existing
useTableState,useSearch, andusePaginationhooks for URL state synchronization - Updated Cypress tests to work with new Ant Design table selectors
- Renamed
PropertyActions.tsxtoPropertyActionsCell.tsxfor clarity - Removed unused imports and simplified component structure
- Added consistent sorting to messaging templates API response for predictable table ordering
Confidence Score: 5/5
- This PR is safe to merge with minimal risk
- The changes are well-structured UI migrations that follow established patterns in the codebase. The code properly uses existing hooks for state management (
useTableState,useSearch,usePagination), maintains URL synchronization, and includes comprehensive test updates. Only one minor style improvement found. - No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| clients/admin-ui/src/features/properties/PropertiesTable.tsx | 5/5 | Simplified component to use new custom hook, clean migration to Ant Design |
| clients/admin-ui/src/features/properties/usePropertiesTable.tsx | 5/5 | New hook extracted table logic, uses useTableState for URL state synchronization |
| clients/admin-ui/src/features/properties/PropertyActionsCell.tsx | 4/5 | Renamed from PropertyActions, uses Ant Design components instead of Chakra |
| clients/admin-ui/src/pages/notifications/templates/index.tsx | 5/5 | Migrated to Ant Design Table, uses useTableState hook for state management |
| clients/admin-ui/src/pages/settings/domain-records.tsx | 5/5 | Migrated to Ant Design Table, simplified component structure |
clients/admin-ui/src/features/properties/PropertyActionsCell.tsx
Outdated
Show resolved
Hide resolved
…b.com/ethyca/fides into jpople/eng-708/small-tables-migration
speaker-ender
left a comment
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.
Small comment but otherwise looks good!
If the current design/behavior is correct I can approve.
…b.com/ethyca/fides into jpople/eng-708/small-tables-migration
Ticket ENG-708
Description Of Changes
Migrates properties table, domain verification table, and messaging templates tables to Ant Design.
Code Changes
Steps to Confirm
Pre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works