design(repositories): Refactor repository listing UI#115534
Conversation
github-actions
Bot
added
the
Scope: Frontend
📊 Type Coverage Diff✅ No new type safety issues introduced. Coverage: 93.56% |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
Standard practice for commit messages in this repo can be found here https://develop.sentry.dev/engineering-practices/commit-messages/, since we squash and merge PRs by default this becomes most relevant in the context of the PR title, which will in turn become the commit message on master. If you are creating the PR / committing with claude it should be using Sentry skills / AGENTS.md / CLAUDE.md to follow those practices by default. Looks like some tests need to be updated. |
| <RowButton | ||
| <Stack | ||
| role="region" | ||
| aria-label={merged.integration.name} |
There was a problem hiding this comment.
This was previously labeled with provider.name (e.g. GitHub). It's now just the integration name (e.g. @my-org), so a screen reader navigating by landmarks loses the provider context. Worth combining them, like ${provider.name} ${integration.name}.
| if (visibleRepos.length === 0) { | ||
| return ( | ||
| <Flex padding="xl xl" justify="center" borderTop="primary"> | ||
| {renderEmptyMessage()} | ||
| </Flex> | ||
| ) : ( | ||
| <Grid | ||
| column={outerColumn} | ||
| columns={outerColumns} | ||
| position="relative" | ||
| style={{height: virtualizer.getTotalSize()}} | ||
| > | ||
| {virtualizer.getVirtualItems().map(virtualItem => { | ||
| const repo = visibleRepos[virtualItem.index]!; | ||
| const nameMatch = repoMatches?.[repo.id]?.find(m => m.key === 'name'); | ||
| const isLast = virtualItem.index === visibleRepos.length - 1; | ||
| return ( | ||
| <Fragment key={virtualItem.key}> | ||
| <Container | ||
| column="1/-1" | ||
| position="absolute" | ||
| top="0" | ||
| left="0" | ||
| right="0" | ||
| borderBottom={isLast ? undefined : 'secondary'} | ||
| style={{ | ||
| transform: `translateY(${virtualItem.start}px)`, | ||
| height: virtualItem.size, | ||
| }} | ||
| /> | ||
| <RepoRow | ||
| role="listitem" | ||
| ref={virtualizer.measureElement} | ||
| data-index={virtualItem.index} | ||
| column={contentColumn} | ||
| position="absolute" | ||
| top="0" | ||
| left="0" | ||
| right="0" | ||
| align="center" | ||
| justify="between" | ||
| gap="sm" | ||
| padding={nested ? 'xs xl xs 0' : 'xs lg'} | ||
| style={{transform: `translateY(${virtualItem.start}px)`}} | ||
| > | ||
| <Flex align="center" gap="sm" minWidth="0"> | ||
| <Text> | ||
| {nameMatch | ||
| ? highlightFuseMatches(nameMatch, HighlightMark) | ||
| : repo.name} | ||
| </Text> | ||
| <LinkButton | ||
| className="hover-reveal" | ||
| href={repo.url ?? ''} | ||
| external | ||
| size="zero" | ||
| variant="transparent" | ||
| icon={<IconOpen variant="muted" />} | ||
| aria-label={t('View repository on %s', providerName)} | ||
| tooltipProps={{ | ||
| title: t('View repository on %s', providerName), | ||
| }} | ||
| /> | ||
| </Flex> | ||
| {mappedProjectSlugsByRepoId && ( | ||
| <RepoMappings | ||
| slugs={mappedProjectSlugsByRepoId[repo.id] ?? []} | ||
| mappingsLoading={mappingsLoading} | ||
| action={installation.repoActions?.(repo)} | ||
| /> | ||
| )} | ||
| </RepoRow> | ||
| </Fragment> | ||
| ); | ||
| })} | ||
| </Grid> | ||
| ); | ||
| } |
There was a problem hiding this comment.
The empty/loading/no-match states used to sit inside the role="list" wrapper. This early return is a bare <Flex> outside any list semantics, so "Loading repositories" and "No repositories" aren't announced as part of a list anymore. Could keep the list role on the outer container.
| <Flex column={contentColumn} padding="md xl" justify="center"> | ||
| if (visibleRepos.length === 0) { | ||
| return ( | ||
| <Flex padding="xl xl" justify="center" borderTop="primary"> |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5c9b11b. Configure here.
| <RowButton | ||
| <Stack | ||
| role="region" | ||
| aria-label={merged.integration.name} |
There was a problem hiding this comment.
Region label drops provider
Medium Severity
Each installation card’s role="region" and expand header now use only integration.name for aria-label, so screen readers lose the SCM provider (e.g. GitHub) that used to label the unified provider panel.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5c9b11b. Configure here.
| <Flex column={contentColumn} padding="md xl" justify="center"> | ||
| if (visibleRepos.length === 0) { | ||
| return ( | ||
| <Flex padding="xl xl" justify="center" borderTop="primary"> |
There was a problem hiding this comment.
Empty states lack list role
Medium Severity
When there are no visible repositories, VirtualizedRepoList returns a plain Flex and skips the role="list" container, so loading, no-match, and empty messages are no longer associated with the “Repositories” list landmark.
Reviewed by Cursor Bugbot for commit 5c9b11b. Configure here.
itsdangold
changed the title
| <DropdownMenu | ||
| items={items} | ||
| triggerLabel={t('Connect new provider')} | ||
| triggerLabel={t('Add Integration')} |
There was a problem hiding this comment.
Bug: Multiple tests will fail because they query for UI text labels like 'Connect new provider' and 'Manage repositories' which have been renamed in this PR.
Severity: MEDIUM
Suggested Fix
Update the test files to query for the new UI labels. For example, change screen.getByRole('button', {name: 'Connect new provider'}) to use the new name 'Add Integration', and update screen.getByText('Manage repositories') to look for 'Manage'.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
static/app/views/settings/organizationRepositories/components/connectProviderDropdown.tsx#L59
Potential issue: The pull request updates UI text labels in several components, but the
corresponding test files that assert on these labels have not been updated.
Specifically, the button label 'Connect new provider' was changed to 'Add Integration',
and the link text 'Manage repositories' was shortened to 'Manage'. Test suites that
query for these elements by their old text, such as using `screen.getByRole('button',
{name: 'Connect new provider'})` and `screen.getByText('Manage repositories')`, will now
fail because the elements can no longer be found. This will cause at least eight tests
across three different spec files to fail.
Also affects:
static/app/views/settings/organizationRepositories/components/connectProviderDropdown.spec.tsxstatic/app/views/settings/organizationRepositories/index.spec.tsxstatic/app/views/settings/organizationRepositories/components/scmRepositoryTable.tsxstatic/app/views/settings/organizationRepositories/components/scmRepositoryTable.spec.tsx
2 participants
Making a few adjustments to the repositories list view :)
Changelog:
Layout
SingleInstallTable/MultiInstallTablesplit and the grouped SCM-title panel)<Panel>with<Stack>(border, radius, overflow:hidden)gap="xl")xs→md)xs→sm)md→xl)Card header
tokens.interactive.link.neutral)warning→dangervariantRepo list
PanelHeadercolumn-heading row ("Repositories" / "Connected Projects"); only renders when there are repos to shownestedmode and subgrid rendering inVirtualizedRepoList