refactor(ui): migrate Groups tab and CreateGroupModal to Alchemy components CAT-1546#16541
Conversation
…onents Replace legacy antd and custom components in the Groups identity tab with their Alchemy equivalents to align with the ongoing design system migration. Groups tab (GroupList.tsx): - Replace antd List with Alchemy Table (with inline cell components) - Replace custom SearchBar with Alchemy SearchBar - Replace antd Pagination with Alchemy Pagination - Replace custom avatars/pills/menus with Alchemy Avatar, Pill, Menu - Replace role selector with Alchemy SimpleSelectRole - Add three-dot vertical actions menu (Phosphor DotsThreeVertical) - Move "Create Group" button to page header (dynamic per active tab) - Use semantic theme color tokens instead of hardcoded values CreateGroupModal: - Replace antd Form/Input with Alchemy Input (with built-in labels) - Replace antd Collapse with Alchemy Button toggle for Advanced section - Use Alchemy Label component for Description field - Use semantic theme color tokens Layout fixes: - Fix flex height chain through antd Tabs internals so table content fills available space with pagination pinned at the bottom - Remove conflicting overflow/sticky-header CSS that broke Alchemy Table's native scroll behavior - Remove extra padding from ServiceAccountList containers Made-with: Cursor
|
Linear: CAT-1511 |
|
🔴 Meticulous spotted visual differences in 23 of 1515 screens tested: view and approve differences detected. Meticulous evaluated ~9 hours of user flows against your PR. Last updated for commit e2af4c8. This comment will update as new commits are pushed. |
Bundle ReportChanges will increase total bundle size by 2.71kB (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: datahub-react-web-esmAssets Changed:
Files in
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
maggiehays
added
the
needs-review
…and Cypress selectors - Fix description not showing: use info.description and editableProperties.description instead of non-existent properties.description in listGroups query - Add editableProperties.description to listGroups GraphQL query - Replace Editor with TextArea in CreateGroupModal (rich text wasn't rendering on profile) - Add table-layout: fixed and column width percentages to prevent content overflow - Truncate long descriptions with ellipsis - Change members pill to small gray outline - Show empty cell instead of "-" when no description - Apply flexbox layout chain (PR #16565 pattern) to AlchemyRoutedTabs and Content - Update Cypress tests: fix remove group flow, description selector, button casing Made-with: Cursor
Use displayName instead of group.name in GroupActionsMenu data-testid so the selector reflects edited group names. Wrap createGroup in V2 mode in dataset_ownership.js since the Create Group button now lives in the V2 header. Update delete assertion in v2_managing_groups.js to match the actual dynamic success message. Made-with: Cursor
Made-with: Cursor
…b files Made-with: Cursor
annadoesdesign
force-pushed
the
refactor/groups-tab-alchemy-migration
branch
from
dadaa97 to
a36cd05
Compare
maggiehays
added
needs-review
annadoesdesign
changed the title
…-alchemy-migration Made-with: Cursor # Conflicts: # datahub-web-react/src/app/entityV2/user/UserProfile.tsx
Update Phosphor icon usage in group identity components to pass direct component references instead of string literals, aligning with the new Icon API from #16338. Made-with: Cursor
| const AdvancedButton = styled(Button)` | ||
| padding-left: 0; | ||
| padding-right: 0; | ||
| color: ${(props) => props.theme.colors.textSecondary}; |
There was a problem hiding this comment.
Ideally we should refactor styles into .components. file (especially for larger files).
| {groups.length > 0 ? ( | ||
| <> | ||
| <Table columns={columns} data={groups} isLoading={loading} isScrollable /> | ||
| <div style={{ paddingTop: '8px', display: 'flex', justifyContent: 'center' }}> |
There was a problem hiding this comment.
Nit: we should not be embedding styles inline (not until we switch to Tailwind!)
| const isGroupsTab = activeTab === 'groups'; | ||
|
|
||
| const renderActionButton = () => { | ||
| if (isServiceAccountsTab) { |
There was a problem hiding this comment.
Nit: missed opportunity for a switch!
|
|
||
| setTimeout(async () => { | ||
| clearRoleListCache(client); | ||
| await refetch(); |
There was a problem hiding this comment.
Technically refetch can fail too. We should probably try/catch (use .then() without async/await).
| newRoleUrn === NO_ROLE_URN | ||
| ? `Failed to remove role from ${entityLabel} ${roleAssignmentState.actorName}: ${e.message || ''}` | ||
| : `Failed to assign role ${roleToAssign?.name} to ${entityLabel} ${roleAssignmentState.actorName}: ${e.message || ''}`, | ||
| ); |
There was a problem hiding this comment.
Instead of repeating ternary logic in toasts we could precalculate the message e.g.
const isRemoval = newRoleUrn === NO_ROLE_URN;
const successMsg = isRemoval ? `Removed role from ...` : `Assigned role ...`;
const failureMsg = isRemoval ? `Failed to remove role from ...` : `Failed to assign role ...`;
|
Thanks for migrating Groups! |
- Extract CreateGroupModal styled components into .components.tsx file - Replace inline styles with PaginationContainer styled component - Use switch statement in ManageUsersAndGroups renderActionButton - Handle refetch errors in useRoleAssignment with .then()/.catch() - Precalculate isRemoval/success/failure toast messages Made-with: Cursor
5 participants
Before:
After:
Summary
GroupList.tsx) from antd/custom components to Alchemy equivalents:Table,SearchBar,Pagination,Avatar,Pill,Menu,SimpleSelectRole, andButton— preserving all existing functionality (CRUD, role assignment, search, onboarding tour)Form/Input/Collapseto AlchemyInput(with built-in labels),Label, andButtontoggle for the Advanced sectionContent→RoutedTabsStyle→StyledTabsPrimary→ tab pane →RouteContainer) so the table fills available space with pagination pinned at the bottom instead of overflowing off-screentheme.colors.textSecondary,theme.colors.border,theme.colors.bg) across all touched filesChecklist
yarn lint(eslint + prettier + type-check) passesTest plan
Made with Cursor