Skip to content

Commit

Permalink
Show display names in the group members table
Browse files Browse the repository at this point in the history
 - Show display names next to username in table if present
 - Prefix username with '@' and make them bold and darker
 - Change table to use grid lines and non-striped rows, to align with the
   designs.
  • Loading branch information
robertknight committed Dec 3, 2024
1 parent 34a15e6 commit facea44
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 12 deletions.
33 changes: 25 additions & 8 deletions h/static/scripts/group-forms/components/EditGroupMembersForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ type TableColumn<Row> = {
};

type MemberRow = {
username: string;
userid: string;

username: string;
displayName?: string;

/** Whether to show the button to remove this member from the group? */
showDeleteAction: boolean;

Expand Down Expand Up @@ -61,6 +63,7 @@ function memberToRow(member: GroupMember, currentUserid: string): MemberRow {
: [role];
return {
userid: member.userid,
displayName: member.display_name,
username: member.username,
showDeleteAction:
member.actions.includes('delete') && member.userid !== currentUserid,
Expand Down Expand Up @@ -253,18 +256,30 @@ export default function EditGroupMembersForm({
switch (field) {
case 'username':
return (
<div
data-testid="username"
className="truncate"
title={user.username}
>
{user.username}
<div className="truncate" title={user.username}>
<span data-testid="username" className="font-bold text-grey-7">
@{user.username}
</span>
{user.displayName && (
<span data-testid="display-name">
{
// Create space using a separate element, rather than eg.
// `inline-block ml-3` on the display name container because
// that would cause the entire display name to be hidden if
// truncated.
<span className="inline-block w-3" />
}
{user.displayName}
</span>
)}
</div>
);
case 'role':
if (user.availableRoles.length <= 1) {
return (
<span data-testid={`role-${user.username}`}>
// Left padding here aligns the static role label in this row with
// the current role in dropdowns in other rows.
<span className="pl-2" data-testid={`role-${user.username}`}>
{roleStrings[user.role]}
</span>
);
Expand Down Expand Up @@ -303,6 +318,8 @@ export default function EditGroupMembersForm({
<div className="w-full">
<Scroll>
<DataTable
grid
striped={false}
title="Group members"
rows={members ?? []}
columns={columns}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ describe('EditGroupMembersForm', () => {
{
userid: 'acct:bob@localhost',
username: 'bob',
display_name: 'Bob Jones',
actions: [
'delete',
'updates.roles.admin',
Expand All @@ -26,6 +27,7 @@ describe('EditGroupMembersForm', () => {
{
userid: 'acct:johnsmith@localhost',
username: 'johnsmith',
display_name: 'John Smith',
actions: [],
roles: ['owner'],
},
Expand Down Expand Up @@ -125,10 +127,16 @@ describe('EditGroupMembersForm', () => {
return wrapper.find('ErrorNotice').prop('message') !== null;
});

const getRenderedUsers = wrapper => {
const getRenderedUsernames = wrapper => {
return wrapper.find('[data-testid="username"]').map(node => node.text());
};

const getRenderedDisplayNames = wrapper => {
return wrapper
.find('[data-testid="display-name"]')
.map(node => node.text());
};

const getRemoveUserButton = (wrapper, username) => {
return wrapper.find(`IconButton[data-testid="remove-${username}"]`);
};
Expand Down Expand Up @@ -182,8 +190,11 @@ describe('EditGroupMembersForm', () => {
await waitForTable(wrapper);

assert.isFalse(wrapper.find('DataTable').prop('loading'));
const users = getRenderedUsers(wrapper);
assert.deepEqual(users, ['bob', 'johnsmith', 'jane']);
const usernames = getRenderedUsernames(wrapper);
assert.deepEqual(usernames, ['@bob', '@johnsmith', '@jane']);

const displayNames = getRenderedDisplayNames(wrapper);
assert.deepEqual(displayNames, ['Bob Jones', 'John Smith']);
});

it('displays error if member fetch fails', async () => {
Expand Down Expand Up @@ -282,7 +293,7 @@ describe('EditGroupMembersForm', () => {
assert.equal(error.prop('message'), 'User not found');

// Controls should be re-enabled after saving fails.
assert.include(getRenderedUsers(wrapper), 'bob');
assert.include(getRenderedUsernames(wrapper), '@bob');
assert.isFalse(controlsDisabled(wrapper, 'bob'));
});

Expand Down
1 change: 1 addition & 0 deletions h/static/scripts/group-forms/utils/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export type CreateUpdateGroupAPIResponse = {

export type GroupMember = {
userid: string;
display_name?: string;
username: string;
actions: string[];
roles: Role[];
Expand Down

0 comments on commit facea44

Please sign in to comment.