feat: 8475 Add display name to workspace member list #41452
Conversation
Adds display name to the workspace member list.
WalkthroughA new Display Name column is added to the Members page, featuring a styled Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/client/src/ce/pages/workspace/Members.tsx (3)
71-78: nth-child-based header widths are fragile to column-order changesThe new
:nth-child(2)/:nth-child(3)rules tightly couple widths to the current DOM column order. If a selection column is added, columns are reordered, or some column is conditionally hidden, these widths may apply to the wrong headers.If the
TableAPI allows, prefer setting widths via column definitions (e.g.,width/minWidth) or via a header className rather than structural selectors. Otherwise, this is fine but worth keeping in mind when touching this table again.
71-78: Avoid duplicating the Display Name column width magic number
140pxis now baked into both the header (th:nth-child(2)) andDisplayNameText.max-width, so a future tweak risks drifting these apart.You can centralize this width in a constant and reuse it in both styled components:
+const DISPLAY_NAME_COL_WIDTH = "140px"; + export const MembersWrapper = styled.div<{ isMobile?: boolean; }>` @@ - &:nth-child(2) { - width: 140px; - } + &:nth-child(2) { + width: ${DISPLAY_NAME_COL_WIDTH}; + } @@ -export const DisplayNameText = styled.div` - max-width: 140px; +export const DisplayNameText = styled.div` + max-width: ${DISPLAY_NAME_COL_WIDTH}; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; color: var(--ads-v2-color-fg); `;This keeps layout adjustments for the Display Name column in sync.
Also applies to: 200-206
373-388: Consider search highlighting and mobile parity for Display NameThe new Display Name column works, but a couple of UX tweaks are worth considering:
- Highlight search matches in Display Name, similar to the username column:
- Cell: function DisplayNameCell(props: any) { + Cell: function DisplayNameCell(props: any) { const member = props.cell.row.original; const displayName = member.name || "-"; return ( <RowWrapper> - <DisplayNameText title={displayName}>{displayName}</DisplayNameText> + <DisplayNameText title={displayName}> + <HighlightText highlight={searchValue} text={displayName} /> + </DisplayNameText> </RowWrapper> ); },
- Mobile card still doesn’t surface
member.name: if the feature requires “Display Name” across viewports, you may also want to showmember.namein the mobile card (e.g., usemember.name || member.usernameas the primary label) to keep desktop and mobile behavior consistent.Both are incremental changes that improve discoverability without impacting existing logic.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/ce/pages/workspace/Members.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-lint / client-lint
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/20049395235. |
ankitakinger
changed the title
|
Deploy-Preview-URL: https://ce-41452.dp.appsmith.com |
| &:nth-child(3) { | ||
| width: 110px; | ||
| } | ||
There was a problem hiding this comment.
Remove:
&:nth-child(2) {
width: 140px;
}
&:nth-child(3) {
width: 110px;
}
Add:
&:first-child {
width: 240px;
}
2 participants
Adds display name to the workspace member list.
Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Adds Display Name to Workspace
Fixes 8475
Fixes https://github.com/appsmithorg/appsmith-ee/issues/8475
Automation
/ok-to-test tags=""
🔍 Cypress test results
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?