-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
fix(replay): Iterate on the bulk "Delete" replay button when its disabled #101498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| import {Fragment} from 'react'; | ||
| import styled from '@emotion/styled'; | ||
|
|
||
| import {Tooltip} from '@sentry/scraps/tooltip/tooltip'; | ||
|
|
||
| import {Alert} from 'sentry/components/core/alert'; | ||
| import {Flex} from 'sentry/components/core/layout/flex'; | ||
| import DeleteReplays from 'sentry/components/replays/table/deleteReplays'; | ||
|
|
@@ -13,6 +15,11 @@ import {t, tct, tn} from 'sentry/locale'; | |
| import type {Sort} from 'sentry/utils/discover/fields'; | ||
| import {useListItemCheckboxContext} from 'sentry/utils/list/useListItemCheckboxState'; | ||
| import {parseQueryKey} from 'sentry/utils/queryClient'; | ||
| import {decodeList} from 'sentry/utils/queryString'; | ||
| import useDeleteReplayHasAccess from 'sentry/utils/replays/hooks/useDeleteReplayHasAccess'; | ||
| import useLocationQuery from 'sentry/utils/url/useLocationQuery'; | ||
| import useProjectFromId from 'sentry/utils/useProjectFromId'; | ||
| import useProjects from 'sentry/utils/useProjects'; | ||
| import type {ReplayListRecord} from 'sentry/views/replays/types'; | ||
|
|
||
| type Props = { | ||
|
|
@@ -29,6 +36,35 @@ export default function ReplayTableHeader({columns, replays, onSortClick, sort}: | |
| const queryOptions = parseQueryKey(queryKey).options; | ||
| const queryString = queryOptions?.query?.query; | ||
|
|
||
| const {project: selectedProjectIds} = useLocationQuery({ | ||
| fields: { | ||
| project: decodeList, | ||
| }, | ||
| }); | ||
| const {projects} = useProjects(); | ||
| const hasOnlyOneProject = projects.length === 1; | ||
|
|
||
| // if 1 project is selected, use it | ||
| // if no project is selected but only 1 project exists, use that | ||
| const project = useProjectFromId({ | ||
| project_id: | ||
| selectedProjectIds.length === 1 | ||
| ? selectedProjectIds[0] | ||
| : hasOnlyOneProject | ||
| ? projects[0]?.id | ||
| : undefined, | ||
| }); | ||
|
|
||
| const hasAccess = useDeleteReplayHasAccess({project}); | ||
| const hasOneProjectSelected = Boolean(project); | ||
| const oneProjectEligible = hasOneProjectSelected || hasOnlyOneProject; | ||
|
|
||
| const disabledMessage = oneProjectEligible | ||
| ? hasAccess | ||
| ? undefined | ||
| : t('You must have project:write or project:admin access to delete replays') | ||
| : t('Select a single project from the dropdown to delete replays'); | ||
|
|
||
| return ( | ||
| <Fragment> | ||
| <TableHeader> | ||
|
|
@@ -57,18 +93,24 @@ export default function ReplayTableHeader({columns, replays, onSortClick, sort}: | |
| /> | ||
| </TableCellFirst> | ||
| <TableCellsRemaining> | ||
| <DeleteReplays | ||
| queryOptions={queryOptions} | ||
| replays={replays} | ||
| selectedIds={selectedIds} | ||
| /> | ||
| <Tooltip disabled={!disabledMessage} title={disabledMessage}> | ||
| {project ? ( | ||
| <DeleteReplays | ||
| disabled={!oneProjectEligible || !hasAccess} | ||
| project={project} | ||
| queryOptions={queryOptions} | ||
| replays={replays} | ||
| selectedIds={selectedIds} | ||
| /> | ||
| ) : null} | ||
| </Tooltip> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Button and Tooltip Vanish on Null ProjectThe |
||
| </TableCellsRemaining> | ||
| </TableHeader> | ||
| ) : null} | ||
|
|
||
| {isAllSelected === 'indeterminate' ? ( | ||
| <FullGridAlert type="warning" system> | ||
| <Flex justify="center" wrap="wrap" gap="md"> | ||
| <Flex justify="center" wrap="wrap" gap="sm"> | ||
| {tn( | ||
| 'Selected %s visible replay.', | ||
| 'Selected %s visible replays.', | ||
|
|
@@ -87,21 +129,19 @@ export default function ReplayTableHeader({columns, replays, onSortClick, sort}: | |
|
|
||
| {isAllSelected === true ? ( | ||
| <FullGridAlert type="warning" system> | ||
| <Flex justify="center" wrap="wrap"> | ||
| <span> | ||
| {queryString | ||
| ? tct('Selected all replays matching: [queryString].', { | ||
| queryString: <var>{queryString}</var>, | ||
| }) | ||
| : countSelected > replays.length | ||
| ? t('Selected all %s+ replays.', replays.length) | ||
| : tn( | ||
| 'Selected all %s replay.', | ||
| 'Selected all %s replays.', | ||
| countSelected | ||
| )} | ||
| </span> | ||
| </Flex> | ||
| {queryString | ||
| ? tct('Selected all replays matching: [queryString].', { | ||
| queryString: <var>{queryString}</var>, | ||
| }) | ||
| : countSelected > replays.length | ||
| ? t('Selected all %s+ replays.', replays.length) | ||
| : tn('Selected all %s replay.', 'Selected all %s replays.', countSelected)} | ||
| </FullGridAlert> | ||
| ) : null} | ||
|
|
||
| {isAllSelected && disabledMessage ? ( | ||
| <FullGridAlert type="error" system> | ||
| {disabledMessage} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Permission Banner Missing for Partial SelectionThe permission error banner for deleting replays only appears when all replays are selected. When only some replays are selected, the delete button is disabled, but the banner explaining the lack of permission doesn't show up, leaving users without an explanation for the disabled action. |
||
| </FullGridAlert> | ||
| ) : null} | ||
| </Fragment> | ||
|
|
@@ -118,6 +158,7 @@ const TableCellFirst = styled(SimpleTable.HeaderCell)` | |
| `; | ||
|
|
||
| const TableCellsRemaining = styled('div')` | ||
| margin-left: ${p => p.theme.space.lg}; | ||
| display: flex; | ||
| align-items: center; | ||
| flex: 1; | ||
|
|
@@ -126,4 +167,5 @@ const TableCellsRemaining = styled('div')` | |
|
|
||
| const FullGridAlert = styled(Alert)` | ||
| grid-column: 1 / -1; | ||
| text-align: center; | ||
| `; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import {hasEveryAccess} from 'sentry/components/acl/access'; | ||
| import type {Project} from 'sentry/types/project'; | ||
| import useOrganization from 'sentry/utils/useOrganization'; | ||
|
|
||
| interface Props { | ||
| project: Project | null | undefined; | ||
| } | ||
|
|
||
| export default function useDeleteReplayHasAccess({project}: Props) { | ||
| const organization = useOrganization(); | ||
|
|
||
| return ( | ||
| hasEveryAccess(['project:write'], {organization, project}) || | ||
| hasEveryAccess(['project:admin'], {organization, project}) | ||
| ); | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabled delete button now disappears when banner is "Select a single project from the dropdown to delete replays", but not when banner is "You must have project:write or project:admin access to delete replays" -- is this intentional?