ref(screenshots): Add design updates - (#34958)

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

@@ -10,12 +10,14 @@ type Props = Omit<ViewerProps, 'attachment'> & {
   attachment: Omit<ViewerProps['attachment'], 'event_id'> & {
     event_id?: string;
   };
+  onError?: React.ReactEventHandler<HTMLImageElement>;
+  onLoad?: React.ReactEventHandler<HTMLImageElement>;
 };
 
-function ImageViewer({className, ...props}: Props) {
+function ImageViewer({className, onLoad, onError, ...props}: Props) {
   return (
     <Container className={className}>
-      <img src={getAttachmentUrl(props, true)} />
+      <img src={getAttachmentUrl(props, true)} onLoad={onLoad} onError={onError} />
     </Container>
   );
 }

static/app/components/events/contextSummary/item.tsx

@@ -34,11 +34,10 @@ const Wrapper = styled('div')`
   position: relative;
   min-width: 0;
 
-  :not(:last-child) {
-    margin-right: ${space(3)};
-  }
-
   @media (min-width: ${p => p.theme.breakpoints[0]}) {
+    :not(:last-child) {
+      margin-right: ${space(3)};
+    }
     max-width: 25%;
     border: 0;
     padding: 0px 0px 0px 42px;

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

@@ -31,6 +31,7 @@ function DataSection({title, description, children, className, ...props}: Props)
         </TitleWrapper>
       }
       wrapTitle={false}
+      showPermalink={false}
     >
       {children}
     </StyledEventDataSection>
@@ -57,11 +58,4 @@ const StyledEventDataSection = styled(EventDataSection)`
   ${SectionContents} {
     flex: 1;
   }
-
-  @media (min-width: ${p => p.theme.breakpoints[0]}) {
-    && {
-      padding: 0;
-      border: 0;
-    }
-  }
 `;

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

@@ -1,3 +1,4 @@
+import {css} from '@emotion/react';
 import styled from '@emotion/styled';
 
 import {DataSection} from 'sentry/components/events/styles';
@@ -38,9 +39,16 @@ function EventTagsAndScreenshots({
     return null;
   }
 
+  const showScreenshot = !isShare && !!screenshot;
+  const showTags = !!tags.length || hasContext;
+
   return (
-    <Wrapper isBorderless={isBorderless}>
-      {!isShare && !!screenshot && (
+    <Wrapper
+      isBorderless={isBorderless}
+      showScreenshot={showScreenshot}
+      showTags={showTags}
+    >
+      {showScreenshot && (
         <Screenshot
           organization={organization}
           event={event}
@@ -49,7 +57,8 @@ function EventTagsAndScreenshots({
           onDelete={onDeleteScreenshot}
         />
       )}
-      {(!!tags.length || hasContext) && (
+      {showScreenshot && showTags && <Divider />}
+      {showTags && (
         <Tags
           organization={organization}
           event={event}
@@ -64,37 +73,55 @@ function EventTagsAndScreenshots({
 
 export default EventTagsAndScreenshots;
 
-const Wrapper = styled(DataSection)<{isBorderless: boolean}>`
-  display: grid;
-  gap: ${space(3)};
-
-  @media (max-width: ${p => p.theme.breakpoints[0]}) {
-    && {
-      padding: 0;
+const Wrapper = styled(DataSection)<{
+  isBorderless: boolean;
+  showScreenshot: boolean;
+  showTags: boolean;
+}>`
+  > * {
+    :first-child,
+    :last-child {
       border: 0;
+      padding: 0;
     }
   }
 
   @media (min-width: ${p => p.theme.breakpoints[0]}) {
-    padding-bottom: ${space(2)};
-    grid-template-columns: 1fr auto;
-    gap: ${space(4)};
-
-    > *:first-child {
-      border-bottom: 0;
-      padding-bottom: 0;
+    display: grid;
+    grid-template-columns: ${p =>
+      p.showScreenshot && p.showTags ? 'max-content auto 1fr' : '1fr'};
+    padding-top: 0;
+    padding-bottom: 0;
+    && {
+      > * {
+        :first-child,
+        :last-child {
+          border: 0;
+          padding: ${space(3)} 0;
+        }
+      }
     }
   }
 
   ${p =>
     p.isBorderless &&
-    `
-    && {
-        padding: ${space(3)} 0 0 0;
-        :first-child {
-          padding-top: 0;
-          border-top: 0;
-        }
+    css`
+      && {
+        padding-left: 0;
+        padding-right: 0;
       }
     `}
 `;
+
+const Divider = styled('div')`
+  background: ${p => p.theme.innerBorder};
+  height: 1px;
+  width: 100%;
+  margin: ${space(3)} 0;
+
+  @media (min-width: ${p => p.theme.breakpoints[0]}) {
+    height: 100%;
+    width: 1px;
+    margin: 0 ${space(3)};
+  }
+`;

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

@@ -8,7 +8,6 @@ const ImageVisualization = styled(ImageViewer)`
   img {
     width: auto;
     height: 100%;
-    object-fit: cover;
     flex: 1;
   }
 `;

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

@@ -1,4 +1,4 @@
-import {Fragment} from 'react';
+import {Fragment, useState} from 'react';
 import styled from '@emotion/styled';
 
 import {openModal} from 'sentry/actionCreators/modal';
@@ -7,6 +7,7 @@ import MenuItemActionLink from 'sentry/components/actions/menuItemActionLink';
 import Button from 'sentry/components/button';
 import ButtonBar from 'sentry/components/buttonBar';
 import DropdownLink from 'sentry/components/dropdownLink';
+import LoadingIndicator from 'sentry/components/loadingIndicator';
 import {Panel, PanelBody, PanelFooter} from 'sentry/components/panels';
 import {IconEllipsis} from 'sentry/icons';
 import {t} from 'sentry/locale';
@@ -29,6 +30,7 @@ type Props = {
 
 function Screenshot({event, organization, screenshot, projectSlug, onDelete}: Props) {
   const orgSlug = organization.slug;
+  const [loadingImage, setLoadingImage] = useState(true);
 
   function handleOpenVisualizationModal(
     eventAttachment: EventAttachment,
@@ -55,13 +57,23 @@ function Screenshot({event, organization, screenshot, projectSlug, onDelete}: Pr
 
     return (
       <Fragment>
-        <StyledPanelBody>
+        <StyledPanelBody
+          onClick={() =>
+            handleOpenVisualizationModal(
+              screenshotAttachment,
+              `${downloadUrl}?download=1`
+            )
+          }
+        >
           <StyledImageVisualization
             attachment={screenshotAttachment}
             orgId={orgSlug}
             projectId={projectSlug}
             event={event}
+            onLoad={() => setLoadingImage(false)}
+            onError={() => setLoadingImage(false)}
           />
+          {loadingImage && <StyledLoadingIndicator mini />}
         </StyledPanelBody>
         <StyledPanelFooter>
           <StyledButtonbar gap={1}>
@@ -99,9 +111,9 @@ function Screenshot({event, organization, screenshot, projectSlug, onDelete}: Pr
                 title={t('Delete')}
                 onAction={() => onDelete(screenshotAttachment.id)}
                 header={t(
-                  'Screenshots help identify what the user saw when the event happened'
+                  'This image was captured around the time that the event occurred.'
                 )}
-                message={t('Are you sure you wish to delete this screenshot?')}
+                message={t('Are you sure you wish to delete this image?')}
               >
                 {t('Delete')}
               </MenuItemActionLink>
@@ -123,7 +135,7 @@ function Screenshot({event, organization, screenshot, projectSlug, onDelete}: Pr
           <DataSection
             title={t('Screenshot')}
             description={t(
-              'Screenshot help identify what the user saw when the event happened'
+              'This image was captured around the time that the event occurred.'
             )}
           >
             <StyledPanel>{renderContent(screenshot)}</StyledPanel>
@@ -142,32 +154,47 @@ const StyledPanel = styled(Panel)`
   justify-content: center;
   align-items: center;
   margin-bottom: 0;
-  min-height: 200px;
-  min-width: 175px;
+  max-width: 100%;
   height: 100%;
+  border: 0;
+
+  @media (min-width: ${p => p.theme.breakpoints[0]}) {
+    max-width: 175px;
+  }
 `;
 
 const StyledPanelBody = styled(PanelBody)`
-  min-height: 175px;
-  height: 100%;
-  overflow: hidden;
   border: 1px solid ${p => p.theme.border};
-  border-radius: ${p => p.theme.borderRadius};
-  margin: -1px;
-  width: calc(100% + 2px);
-  border-bottom-left-radius: 0;
-  border-bottom-right-radius: 0;
+  border-top-left-radius: ${p => p.theme.borderRadius};
+  border-top-right-radius: ${p => p.theme.borderRadius};
+  width: 100%;
+  min-height: 48px;
+  overflow: hidden;
+  cursor: pointer;
   position: relative;
+  display: flex;
+  align-items: center;
+  justify-content: center;
+  flex: 1;
 `;
 
 const StyledPanelFooter = styled(PanelFooter)`
   padding: ${space(1)};
   width: 100%;
+  border: 1px solid ${p => p.theme.border};
+  border-top: 0;
+  border-bottom-left-radius: ${p => p.theme.borderRadius};
+  border-bottom-right-radius: ${p => p.theme.borderRadius};
 `;
 
-const StyledImageVisualization = styled(ImageVisualization)`
+const StyledLoadingIndicator = styled(LoadingIndicator)`
   position: absolute;
+`;
+
+const StyledImageVisualization = styled(ImageVisualization)`
   width: 100%;
+  z-index: 1;
+  border: 0;
 `;
 
 const StyledButtonbar = styled(ButtonBar)`

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

@@ -65,16 +65,11 @@ function Modal({
             )}
           </Value>
 
-          <Label>{t('Name')}</Label>
-          <Value>{t('Screenshot')}</Value>
+          <Label>{t('Size')}</Label>
+          <Value>{defined(size) ? formatBytesBase2(size) : <NotAvailable />}</Value>
 
-          <Label coloredBg>{t('Size')}</Label>
-          <Value coloredBg>
-            {defined(size) ? formatBytesBase2(size) : <NotAvailable />}
-          </Value>
-
-          <Label>{t('MIME Type')}</Label>
-          <Value>{mimetype ?? <NotAvailable />}</Value>
+          <Label coloredBg>{t('MIME Type')}</Label>
+          <Value coloredBg>{mimetype ?? <NotAvailable />}</Value>
         </GeralInfo>
 
         <StyledImageVisualization

static/app/components/questionTooltip.tsx

@@ -41,7 +41,7 @@ function QuestionTooltip({title, size, className, ...tooltipProps}: QuestionProp
   return (
     <QuestionIconContainer size={size} className={className}>
       <Tooltip title={title} {...tooltipProps}>
-        <IconQuestion size={size} />
+        <IconQuestion size={size} data-test-id="more-information" />
       </Tooltip>
     </QuestionIconContainer>
   );

tests/js/spec/components/events/eventTagsAndScreenshot.spec.tsx

@@ -1,7 +1,10 @@
+import {Fragment} from 'react';
+
 import {initializeOrg} from 'sentry-test/initializeOrg';
-import {render, screen, within} from 'sentry-test/reactTestingLibrary';
+import {render, screen, userEvent, within} from 'sentry-test/reactTestingLibrary';
 
 import EventTagsAndScreenshot from 'sentry/components/events/eventTagsAndScreenshot';
+import GlobalModal from 'sentry/components/globalModal';
 import {EventAttachment} from 'sentry/types';
 
 import {deviceNameMapper} from '../../../../../static/app/components/deviceName';
@@ -244,17 +247,35 @@ describe('EventTagsAndScreenshot', function () {
   });
 
   describe('renders screenshot only', function () {
-    it('no context and no tags', function () {
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/repos/',
+      body: [],
+    });
+
+    MockApiClient.addMockResponse({
+      url: '/projects/org-slug/project-slug/releases/io.sentry.sample.iOS-Swift%407.2.3%2B390/',
+      body: {},
+    });
+
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/releases/io.sentry.sample.iOS-Swift%407.2.3%2B390/deploys/',
+      body: [],
+    });
+
+    it('no context and no tags', async function () {
       const {container} = render(
-        <EventTagsAndScreenshot
-          event={event}
-          organization={organization}
-          projectId={project.slug}
-          location={router.location}
-          attachments={attachments}
-          onDeleteScreenshot={() => jest.fn()}
-          hasContext={false}
-        />
+        <Fragment>
+          <GlobalModal />
+          <EventTagsAndScreenshot
+            event={event}
+            organization={organization}
+            projectId={project.slug}
+            location={router.location}
+            attachments={attachments}
+            onDeleteScreenshot={() => jest.fn()}
+            hasContext={false}
+          />
+        </Fragment>
       );
 
       // Tags Container
@@ -268,6 +289,26 @@ describe('EventTagsAndScreenshot', function () {
         `/api/0/projects/${organization.slug}/${project.slug}/events/${event.id}/attachments/${attachments[1].id}/?download`
       );
 
+      // Display help text when hovering question element
+      userEvent.hover(screen.getByTestId('more-information'));
+
+      expect(
+        await screen.findByText(
+          'This image was captured around the time that the event occurred.'
+        )
+      ).toBeInTheDocument();
+
+      // Screenshot is clickable
+      userEvent.click(screen.getByRole('img'));
+
+      // Open 'view screenshot' dialog
+      expect(screen.getByRole('dialog')).toBeInTheDocument();
+      expect(
+        within(screen.getByRole('dialog')).getByText('Screenshot')
+      ).toBeInTheDocument();
+
+      userEvent.click(screen.getByLabelText('Close Modal'));
+
       expect(container).toSnapshot();
     });
   });