ServiceAccount : use InteractiveTable
#85203
Conversation
eledobleefe
added
no-backport
… ServiceAccountsListItem
| ) : ( | ||
| <Stack alignItems="center"> | ||
| <Icon name="key-skeleton-alt"></Icon> | ||
| <TextLink href={href} aria-label={ariaLabel} color="primary"> |
There was a problem hiding this comment.
I don't think this needs to be a link, the first three columns should be enough.
There was a problem hiding this comment.
🤷 I copied it from the original code. It is how it works on main. Maybe a question for the identity team?
public/app/features/serviceaccounts/ServiceAccountsListPage.tsx
Outdated
Show resolved
Hide resolved
Yes, they will be added automatically when the PR turn into a normal one, not a draft. But wanted to check if I was in the right direction 😅 |
| const getCellContent = ( | ||
| value: string, | ||
| original: ServiceAccountDTO, | ||
| isLoading: boolean, | ||
| columnName?: Column<ServiceAccountDTO>['id'] | ||
| ) => { | ||
| if (isLoading) { | ||
| return columnName === 'avatarUrl' ? <Skeleton circle width={24} height={24} /> : <Skeleton width={100} />; | ||
| } | ||
| const href = `/org/serviceaccounts/${original.id}`; | ||
| const ariaLabel = `Edit service account's ${name} details`; | ||
| switch (columnName) { | ||
| case 'avatarUrl': | ||
| return ( | ||
| <a aria-label={ariaLabel} href={href}> | ||
| <Avatar src={value} alt={'User avatar'} /> | ||
| </a> | ||
| ); | ||
| case 'id': | ||
| return ( | ||
| <TextLink href={href} aria-label={ariaLabel} color="secondary"> | ||
| {original.login} | ||
| </TextLink> | ||
| ); | ||
| case 'tokens': | ||
| return ( | ||
| <Stack alignItems="center"> | ||
| <Icon name="key-skeleton-alt"></Icon> | ||
| <TextLink href={href} aria-label={ariaLabel} color="primary"> | ||
| {value || 'No tokens'} | ||
| </TextLink> | ||
| </Stack> | ||
| ); | ||
| default: | ||
| return ( | ||
| <TextLink href={href} aria-label={ariaLabel} color="primary"> | ||
| {value} | ||
| </TextLink> | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Does this need to be inside the component body? I was initially thinking that each cell type would have a separate function, but I think this works from a readability point of view, especially with the loading state.
Co-authored-by: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com>
| cell: ({ cell: { value }, row: { original } }: Cell<'role'>) => { | ||
| return getCellContent(value, original, isLoading, 'avatarUrl'); | ||
| }, | ||
| sortType: 'string', |
There was a problem hiding this comment.
Do you actually need sort type here? I don't think it makes sense to have sorting enabled for this column
There was a problem hiding this comment.
Nope, I took a look at the original table and it does not sort so I think we could keep just sorting by 'account'. Wdyt?
eledobleefe
deleted the
eledobleefe/service-accounts-interactive-table-85000
branch
What is this feature?
We modify the
ServiceAccountto use theInteractiveTableWhy do we need this feature?
To be consistent as
OrgUserTableorUserTablealready use it.Who is this feature for?
[Add information on what kind of user the feature is for.]
Which issue(s) does this PR fix?:
Fixes #85000
Special notes for your reviewer:
Please check that: