Skip to content

Commit 3096a2b

Browse files
feat(screenshots): Update render logic (#93211)
Contributes to #92154 This PR updates our logic for rendering screenshots, essentially removing the extension checks to match what the backend [does](138af3b).
1 parent fa40436 commit 3096a2b

File tree

6 files changed

+68
-51
lines changed

6 files changed

+68
-51
lines changed

static/app/components/events/attachmentViewers/previewAttachmentTypes.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@ import type {IssueAttachment} from 'sentry/types/group';
77

88
export const getInlineAttachmentRenderer = (
99
attachment: IssueAttachment
10-
): typeof ImageViewer | typeof LogFileViewer | typeof RRWebJsonViewer | undefined => {
10+
):
11+
| typeof ImageViewer
12+
| typeof LogFileViewer
13+
| typeof RRWebJsonViewer
14+
| typeof WebMViewer
15+
| undefined => {
1116
switch (attachment.mimetype) {
1217
case 'text/css':
1318
case 'text/csv':

static/app/components/events/attachmentViewers/webmViewer.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,19 @@ import PanelItem from 'sentry/components/panels/panelItem';
66
import {t} from 'sentry/locale';
77

88
interface WebMViewerProps
9-
extends Pick<ViewerProps, 'attachment' | 'eventId' | 'orgSlug' | 'projectSlug'> {}
9+
extends Pick<ViewerProps, 'attachment' | 'eventId' | 'orgSlug' | 'projectSlug'>,
10+
Partial<Pick<HTMLVideoElement, 'controls'>> {
11+
onCanPlay?: React.ReactEventHandler<HTMLVideoElement>;
12+
}
1013

11-
export function WebMViewer(props: WebMViewerProps) {
14+
export function WebMViewer({controls = true, onCanPlay, ...props}: WebMViewerProps) {
1215
return (
1316
<PanelItem>
1417
<video
15-
controls
18+
onCanPlay={onCanPlay}
19+
controls={controls}
1620
css={css`
21+
width: 100%;
1722
max-width: 100%;
1823
`}
1924
>

static/app/components/events/eventTagsAndScreenshot/index.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,8 @@ export function EventTagsAndScreenshot({projectSlug, event, isShare = false}: Pr
2222
},
2323
{enabled: !isShare}
2424
);
25-
const screenshots =
26-
attachments?.filter(
27-
({name}) =>
28-
name.includes('screenshot') && (name.endsWith('.jpg') || name.endsWith('.png'))
29-
) ?? [];
25+
26+
const screenshots = attachments?.filter(({name}) => name.includes('screenshot')) ?? [];
3027

3128
if (!tags.length && (isShare || !screenshots.length)) {
3229
return null;

static/app/components/events/eventTagsAndScreenshot/screenshot/index.tsx

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {openConfirmModal} from 'sentry/components/confirm';
88
import {Button} from 'sentry/components/core/button';
99
import {ButtonBar} from 'sentry/components/core/button/buttonBar';
1010
import {DropdownMenu} from 'sentry/components/dropdownMenu';
11+
import {getInlineAttachmentRenderer} from 'sentry/components/events/attachmentViewers/previewAttachmentTypes';
1112
import LoadingIndicator from 'sentry/components/loadingIndicator';
1213
import Panel from 'sentry/components/panels/panel';
1314
import PanelBody from 'sentry/components/panels/panelBody';
@@ -22,8 +23,6 @@ import type {Organization} from 'sentry/types/organization';
2223
import type {Project} from 'sentry/types/project';
2324
import {trackAnalytics} from 'sentry/utils/analytics';
2425

25-
import ImageVisualization from './imageVisualization';
26-
2726
type Props = {
2827
eventId: Event['id'];
2928
onDelete: (attachmentId: EventAttachment['id']) => void;
@@ -38,6 +37,14 @@ type Props = {
3837
onlyRenderScreenshot?: boolean;
3938
};
4039

40+
const loadingMimeTypes = [
41+
'image/jpeg',
42+
'image/png',
43+
'image/gif',
44+
'image/webp',
45+
'video/webm',
46+
];
47+
4148
function Screenshot({
4249
eventId,
4350
organization,
@@ -51,8 +58,9 @@ function Screenshot({
5158
onDelete,
5259
openVisualizationModal,
5360
}: Props) {
54-
const orgSlug = organization.slug;
55-
const [loadingImage, setLoadingImage] = useState(true);
61+
const [loadingImage, setLoadingImage] = useState(
62+
loadingMimeTypes.includes(screenshot.mimetype)
63+
);
5664
const {hasRole} = useRole({role: 'attachmentsRole'});
5765

5866
function handleDelete(screenshotAttachmentId: string) {
@@ -62,8 +70,14 @@ function Screenshot({
6270
onDelete(screenshotAttachmentId);
6371
}
6472

73+
if (!hasRole) {
74+
return null;
75+
}
76+
6577
function renderContent(screenshotAttachment: EventAttachment) {
66-
const downloadUrl = `/api/0/projects/${organization.slug}/${projectSlug}/events/${eventId}/attachments/${screenshotAttachment.id}/`;
78+
const AttachmentComponent = getInlineAttachmentRenderer(screenshotAttachment)!;
79+
80+
const downloadUrl = `/api/0/projects/${organization.slug}/${projectSlug}/events/${eventId}/attachments/${screenshot.id}/`;
6781

6882
return (
6983
<Fragment>
@@ -95,20 +109,22 @@ function Screenshot({
95109
<LoadingIndicator mini />
96110
</StyledLoadingIndicator>
97111
)}
98-
<StyledImageWrapper
112+
<AttachmentComponentWrapper
99113
onClick={() =>
100-
openVisualizationModal(screenshotAttachment, `${downloadUrl}?download=1`)
114+
openVisualizationModal(screenshot, `${downloadUrl}?download=1`)
101115
}
102116
>
103-
<StyledImageVisualization
104-
attachment={screenshotAttachment}
105-
orgSlug={orgSlug}
117+
<AttachmentComponent
118+
orgSlug={organization.slug}
106119
projectSlug={projectSlug}
107120
eventId={eventId}
121+
attachment={screenshot}
108122
onLoad={() => setLoadingImage(false)}
109123
onError={() => setLoadingImage(false)}
124+
controls={false}
125+
onCanPlay={() => setLoadingImage(false)}
110126
/>
111-
</StyledImageWrapper>
127+
</AttachmentComponentWrapper>
112128
</StyledPanelBody>
113129
{!onlyRenderScreenshot && (
114130
<StyledPanelFooter>
@@ -240,14 +256,14 @@ const StyledLoadingIndicator = styled('div')`
240256
height: 100%;
241257
`;
242258

243-
const StyledImageWrapper = styled('div')`
259+
const AttachmentComponentWrapper = styled('div')`
244260
:hover {
245261
cursor: pointer;
246262
}
247-
`;
248-
249-
const StyledImageVisualization = styled(ImageVisualization)`
250-
width: 100%;
251-
z-index: 1;
252-
border: 0;
263+
& > * {
264+
width: 100%;
265+
z-index: 1;
266+
border: 0;
267+
padding: 0 !important;
268+
}
253269
`;

static/app/components/events/eventTagsAndScreenshot/screenshot/modal.tsx

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {Button} from 'sentry/components/core/button';
1010
import {ButtonBar} from 'sentry/components/core/button/buttonBar';
1111
import {LinkButton} from 'sentry/components/core/button/linkButton';
1212
import {DateTime} from 'sentry/components/dateTime';
13+
import {getInlineAttachmentRenderer} from 'sentry/components/events/attachmentViewers/previewAttachmentTypes';
1314
import {KeyValueData} from 'sentry/components/keyValueData';
1415
import {t, tct} from 'sentry/locale';
1516
import {space} from 'sentry/styles/space';
@@ -20,7 +21,6 @@ import {formatBytesBase2} from 'sentry/utils/bytes/formatBytesBase2';
2021
import {useHotkeys} from 'sentry/utils/useHotkeys';
2122
import useOrganization from 'sentry/utils/useOrganization';
2223

23-
import ImageVisualization from './imageVisualization';
2424
import ScreenshotPagination from './screenshotPagination';
2525

2626
interface ScreenshotModalProps extends ModalRenderProps {
@@ -58,9 +58,7 @@ export default function ScreenshotModal({
5858
}: ScreenshotModalProps) {
5959
const organization = useOrganization();
6060

61-
const screenshots = attachments.filter(
62-
({name, mimetype}) => name.includes('screenshot') && mimetype.startsWith('image')
63-
);
61+
const screenshots = attachments.filter(({name}) => name.includes('screenshot'));
6462

6563
const [currentEventAttachment, setCurrentAttachment] =
6664
useState<EventAttachment>(eventAttachment);
@@ -108,6 +106,8 @@ export default function ScreenshotModal({
108106
};
109107
}
110108

109+
const AttachmentComponent = getInlineAttachmentRenderer(currentEventAttachment)!;
110+
111111
return (
112112
<Fragment>
113113
<Header closeButton>
@@ -116,12 +116,14 @@ export default function ScreenshotModal({
116116
<Body>
117117
<Flex column gap={space(1.5)}>
118118
{defined(paginationProps) && <ScreenshotPagination {...paginationProps} />}
119-
<StyledImageVisualization
120-
attachment={currentEventAttachment}
121-
orgSlug={organization.slug}
122-
projectSlug={projectSlug}
123-
eventId={currentEventAttachment.event_id}
124-
/>
119+
<AttachmentComponentWrapper>
120+
<AttachmentComponent
121+
attachment={currentEventAttachment}
122+
orgSlug={organization.slug}
123+
projectSlug={projectSlug}
124+
eventId={currentEventAttachment.event_id}
125+
/>
126+
</AttachmentComponentWrapper>
125127
<KeyValueData.Card
126128
title={currentEventAttachment.name}
127129
contentItems={[
@@ -179,10 +181,12 @@ export default function ScreenshotModal({
179181
);
180182
}
181183

182-
const StyledImageVisualization = styled(ImageVisualization)`
183-
border-bottom: 0;
184-
img {
184+
const AttachmentComponentWrapper = styled('div')`
185+
& > * {
186+
padding: 0;
187+
border: none;
185188
max-height: calc(100vh - 300px);
189+
box-sizing: border-box;
186190
}
187191
`;
188192

static/app/components/events/eventTagsAndScreenshot/screenshot/screenshotDataSection.tsx

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,6 @@ import {InterimSection} from 'sentry/views/issueDetails/streamline/interimSectio
2424
import {Tab, TabPaths} from 'sentry/views/issueDetails/types';
2525
import {useHasStreamlinedUI} from 'sentry/views/issueDetails/utils';
2626

27-
const SCREENSHOT_NAMES = [
28-
'screenshot.jpg',
29-
'screenshot.png',
30-
'screenshot-1.jpg',
31-
'screenshot-1.png',
32-
'screenshot-2.jpg',
33-
'screenshot-2.png',
34-
];
35-
3627
interface ScreenshotDataSectionProps {
3728
event: Event;
3829
projectSlug: Project['slug'];
@@ -57,8 +48,7 @@ export function ScreenshotDataSection({
5748
{enabled: !isShare}
5849
);
5950
const {mutate: deleteAttachment} = useDeleteEventAttachmentOptimistic();
60-
const screenshots =
61-
attachments?.filter(({name}) => SCREENSHOT_NAMES.includes(name)) ?? [];
51+
const screenshots = attachments?.filter(({name}) => name.includes('screenshot')) ?? [];
6252

6353
const [screenshotInFocus, setScreenshotInFocus] = useState<number>(0);
6454

0 commit comments

Comments
 (0)