feat(feedback): Refactor the feedback screenshot rendering to show N …

…screenshots side by side (#68136) Related to #67707 Here are some examples of one or more attached screenshots, with different sizes: | Desc | Img | | --- | --- | | single small | ![single small](https://github.com/getsentry/sentry/assets/187460/9b4051e5-0a4e-4c58-bf9d-4e083b7e3cbe) | two same size | ![double same size](https://github.com/getsentry/sentry/assets/187460/e09a7ac7-cc4a-431a-94eb-39a02f31e3ae) | many wrapped | ![many wrapped](https://github.com/getsentry/sentry/assets/187460/14e9c930-093c-49f2-9565-3f602f196750) | many inline | ![many inline](https://github.com/getsentry/sentry/assets/187460/0db6c0f3-1a27-40cf-9e12-608123d2449f)
getsentry · Apr 3, 2024 · 841907c · 841907c
1 parent ea80fe5
commit 841907c
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 181 deletions.
diff --git a/static/app/components/feedback/feedbackItem/feedbackScreenshot.tsx b/static/app/components/feedback/feedbackItem/feedbackScreenshot.tsx
@@ -1,107 +1,57 @@
-import type {ReactEventHandler} from 'react';
-import {Fragment, useState} from 'react';
+import {useState} from 'react';
 import styled from '@emotion/styled';
 
 import {Role} from 'sentry/components/acl/role';
-import {Button} from 'sentry/components/button';
 import ImageVisualization from 'sentry/components/events/eventTagsAndScreenshot/screenshot/imageVisualization';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import Panel from 'sentry/components/panels/panel';
-import PanelBody from 'sentry/components/panels/panelBody';
-import PanelHeader from 'sentry/components/panels/panelHeader';
-import {IconChevron} from 'sentry/icons';
-import {t, tct} from 'sentry/locale';
-import {space} from 'sentry/styles/space';
-import type {Event, EventAttachment, Organization, Project} from 'sentry/types';
+import type {EventAttachment, Organization, Project} from 'sentry/types';
 
 type Props = {
-  eventId: Event['id'];
-  openVisualizationModal: (eventAttachment: EventAttachment) => void;
+  onClick: () => void;
   organization: Organization;
   projectSlug: Project['slug'];
   screenshot: EventAttachment;
-  screenshotInFocus: number;
-  totalScreenshots: number;
-  onNext?: ReactEventHandler;
-  onPrevious?: ReactEventHandler;
 };
 
-function FeedbackScreenshot({
-  eventId,
+export default function FeedbackScreenshot({
   organization,
   screenshot,
-  screenshotInFocus,
-  onNext,
-  onPrevious,
-  totalScreenshots,
   projectSlug,
-  openVisualizationModal,
+  onClick,
 }: Props) {
-  const orgSlug = organization.slug;
-  const [loadingImage, setLoadingImage] = useState(true);
-
-  function renderContent(screenshotAttachment: EventAttachment) {
-    return (
-      <Fragment>
-        {totalScreenshots > 1 && (
-          <StyledPanelHeader lightText>
-            <Button
-              disabled={screenshotInFocus === 0}
-              aria-label={t('Previous Screenshot')}
-              onClick={onPrevious}
-              icon={<IconChevron direction="left" />}
-              size="xs"
-            />
-            {tct('[currentScreenshot] of [totalScreenshots]', {
-              currentScreenshot: screenshotInFocus + 1,
-              totalScreenshots,
-            })}
-            <Button
-              disabled={screenshotInFocus + 1 === totalScreenshots}
-              aria-label={t('Next Screenshot')}
-              onClick={onNext}
-              icon={<IconChevron direction="right" />}
-              size="xs"
-            />
-          </StyledPanelHeader>
-        )}
-        <StyledPanelBody hasHeader={totalScreenshots > 1}>
-          {loadingImage && (
-            <StyledLoadingIndicator>
-              <LoadingIndicator mini />
-            </StyledLoadingIndicator>
-          )}
-          <StyledImageWrapper
-            onClick={() => openVisualizationModal(screenshotAttachment)}
-          >
-            <StyledImageVisualization
-              attachment={screenshotAttachment}
-              orgId={orgSlug}
-              projectSlug={projectSlug}
-              eventId={eventId}
-              onLoad={() => setLoadingImage(false)}
-              onError={() => setLoadingImage(false)}
-            />
-          </StyledImageWrapper>
-        </StyledPanelBody>
-      </Fragment>
-    );
-  }
+  const [isLoading, setIsLoading] = useState(true);
 
   return (
     <Role organization={organization} role={organization.attachmentsRole}>
       {({hasRole}) => {
         if (!hasRole) {
           return null;
         }
-        return <StyledPanel>{renderContent(screenshot)}</StyledPanel>;
+        return (
+          <StyledPanel>
+            {isLoading && (
+              <StyledLoadingIndicator>
+                <LoadingIndicator mini />
+              </StyledLoadingIndicator>
+            )}
+            <StyledImageButton onClick={onClick}>
+              <StyledImageVisualization
+                attachment={screenshot}
+                orgId={organization.slug}
+                projectSlug={projectSlug}
+                eventId={screenshot.event_id}
+                onLoad={() => setIsLoading(false)}
+                onError={() => setIsLoading(false)}
+              />
+            </StyledImageButton>
+          </StyledPanel>
+        );
       }}
     </Role>
   );
 }
 
-export default FeedbackScreenshot;
-
 const StyledPanel = styled(Panel)`
   display: flex;
   flex-direction: column;
@@ -111,38 +61,7 @@ const StyledPanel = styled(Panel)`
   max-width: 360px;
   max-height: 360px;
   border: 0;
-`;
-
-const StyledPanelHeader = styled(PanelHeader)`
-  padding: ${space(1)};
-  width: 100%;
-  border: 1px solid ${p => p.theme.border};
-  border-bottom: 0;
-  border-top-left-radius: ${p => p.theme.borderRadius};
-  border-top-right-radius: ${p => p.theme.borderRadius};
-  display: flex;
-  justify-content: space-between;
-  text-transform: none;
-  background: ${p => p.theme.background};
-`;
-
-const StyledPanelBody = styled(PanelBody)<{hasHeader: boolean}>`
-  border: none;
-  min-height: 48px;
-  overflow: hidden;
-  position: relative;
-  display: flex;
-  flex-direction: column;
-  align-items: center;
-  justify-content: center;
-  flex: 1;
-
-  ${p =>
-    !p.hasHeader &&
-    `
-  border-top-left-radius: ${p.theme.borderRadius};
-  border-top-right-radius: ${p.theme.borderRadius};
-  `}
+  border-radius: ${p => p.theme.borderRadius};
 `;
 
 const StyledLoadingIndicator = styled('div')`
@@ -153,7 +72,7 @@ const StyledLoadingIndicator = styled('div')`
   height: 100%;
 `;
 
-const StyledImageWrapper = styled('button')`
+const StyledImageButton = styled('button')`
   cursor: zoom-in;
   background: none;
   padding: 0;

diff --git a/static/app/components/feedback/feedbackItem/screenshotSection.tsx b/static/app/components/feedback/feedbackItem/screenshotSection.tsx
@@ -1,4 +1,3 @@
-import {useCallback, useState} from 'react';
 import styled from '@emotion/styled';
 
 import {useDeleteEventAttachmentOptimistic} from 'sentry/actionCreators/events';
@@ -15,93 +14,79 @@ import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {Organization} from 'sentry/types';
 import type {Event} from 'sentry/types/event';
-import type {EventAttachment} from 'sentry/types/group';
-import {objectIsEmpty} from 'sentry/utils';
 
 type Props = {
   event: Event;
   organization: Organization;
   projectSlug: string;
 };
 
-export function ScreenshotSection({projectSlug, event, organization}: Props) {
+export function ScreenshotSection({event, organization, projectSlug}: Props) {
   const {screenshots} = useFeedbackScreenshot({projectSlug, event});
-  const hasContext = !objectIsEmpty(event.user ?? {}) || !objectIsEmpty(event.contexts);
   const {mutate: deleteAttachment} = useDeleteEventAttachmentOptimistic();
-  const [screenshotInFocus, setScreenshotInFocus] = useState<number>(0);
 
-  const handleDeleteScreenshot = useCallback(
-    (attachmentId: string) => {
-      deleteAttachment({
-        orgSlug: organization.slug,
-        projectSlug,
-        eventId: event.id,
-        attachmentId,
-      });
-    },
-    [deleteAttachment, event.id, organization.slug, projectSlug]
-  );
-
-  const handleOpenVisualizationModal = useCallback(
-    (eventAttachment: EventAttachment) => {
-      openModal(
-        modalProps => (
-          <OpenScreenshotModal
-            {...modalProps}
-            event={event}
-            orgSlug={organization.slug}
+  return screenshots.length ? (
+    <ScreenshotWrapper>
+      {screenshots.map(screenshot => (
+        <li key={screenshot.id}>
+          <FeedbackScreenshot
+            organization={organization}
             projectSlug={projectSlug}
-            eventAttachment={eventAttachment}
-            attachments={screenshots}
-            attachmentIndex={screenshotInFocus}
+            screenshot={screenshot}
+            onClick={() => {
+              openModal(
+                modalProps => (
+                  <OpenScreenshotModal
+                    {...modalProps}
+                    event={event}
+                    orgSlug={organization.slug}
+                    projectSlug={projectSlug}
+                    eventAttachment={screenshot}
+                    attachments={screenshots}
+                    attachmentIndex={screenshots.indexOf(screenshot)}
+                  />
+                ),
+                {modalCss}
+              );
+            }}
           />
-        ),
-        {modalCss}
-      );
-    },
-    [event, organization.slug, projectSlug, screenshotInFocus, screenshots]
-  );
-
-  if (!hasContext && !screenshots.length) {
-    return null;
-  }
-
-  const showScreenshot = !!screenshots.length;
-  const screenshot = screenshots[screenshotInFocus];
-
-  return showScreenshot ? (
-    <ScreenshotWrapper>
-      <FeedbackScreenshot
-        organization={organization}
-        eventId={event.id}
-        projectSlug={projectSlug}
-        screenshot={screenshot}
-        onNext={() => setScreenshotInFocus(screenshotInFocus + 1)}
-        onPrevious={() => setScreenshotInFocus(screenshotInFocus - 1)}
-        screenshotInFocus={screenshotInFocus}
-        totalScreenshots={screenshots.length}
-        openVisualizationModal={handleOpenVisualizationModal}
-      />
-      <Button
-        icon={<IconDelete />}
-        borderless
-        size="xs"
-        onClick={() => {
-          openConfirmModal({
-            header: t('Delete screenshot?'),
-            message: t('This action cannot be undone.'),
-            confirmText: t('Delete screenshot'),
-            onConfirm: () => handleDeleteScreenshot(screenshot.id),
-            priority: 'danger',
-          });
-        }}
-        aria-label={t('Delete screenshot')}
-      />
+          <Button
+            icon={<IconDelete />}
+            borderless
+            size="xs"
+            onClick={() => {
+              openConfirmModal({
+                header: t('Delete screenshot?'),
+                message: t('This action cannot be undone.'),
+                confirmText: t('Delete screenshot'),
+                onConfirm: () =>
+                  deleteAttachment({
+                    orgSlug: organization.slug,
+                    projectSlug,
+                    eventId: screenshot.event_id,
+                    attachmentId: screenshot.id,
+                  }),
+                priority: 'danger',
+              });
+            }}
+            aria-label={t('Delete screenshot')}
+          />
+        </li>
+      ))}
     </ScreenshotWrapper>
   ) : null;
 }
 
-const ScreenshotWrapper = styled('div')`
+const ScreenshotWrapper = styled('ul')`
   display: flex;
-  gap: ${space(1)};
+  flex-wrap: wrap;
+  gap: ${space(1.5)};
+  margin: 0;
+  padding: 0;
+  list-style: none;
+
+  & > li {
+    display: flex;
+    gap: ${space(1)};
+  }
 `;