Skip to content

Commit

Permalink
ref(feedback): update screenshot display in feedback details (#67955)
Browse files Browse the repository at this point in the history
- Relates to #67707
- Implements some of the design updates from this
[figma](#67707) (see followup
todos at the bottom)
- Updates screenshot display in feedback UI to be part of the purple
message section. Removes extraneous buttons and adds a small delete
button to the right
- Removes extraneous buttons in full screen modal view

## Max dimensions are currently `360px` x `360px`:
<img width="743" alt="SCR-20240329-kqdj"
src="https://github.com/getsentry/sentry/assets/56095982/ba45f3c8-0d21-4824-a3ca-0215bdbb58ae">
<img width="829" alt="SCR-20240329-kqak"
src="https://github.com/getsentry/sentry/assets/56095982/007b9040-d756-4807-98c0-e747a973f848">
<img width="819" alt="SCR-20240329-kpyc"
src="https://github.com/getsentry/sentry/assets/56095982/94a5f111-2618-4798-9b70-73a35bc4022b">

## Full screen / zoom view 
- scales nicely
- zoom-in pointer on hover


https://github.com/getsentry/sentry/assets/56095982/3c97fa45-2109-4f19-9a71-d8e8084397dd



## Screenshot display scales nicely


https://github.com/getsentry/sentry/assets/56095982/0ce3e5b4-b455-4fbf-b48b-280e0a151363

## Delete button with confirmation modal


https://github.com/getsentry/sentry/assets/56095982/79ac0349-18cf-49a3-af77-3b779f00d817

## TODOS
- Currently, if you delete a screenshot, it will not show up in UI
anymore. The design specifies that we should instead show this message
instead:
<img width="396" alt="SCR-20240329-kuoq"
src="https://github.com/getsentry/sentry/assets/56095982/dddf4e47-6f1b-4ac8-88c5-e7b43b50628d">

- Pagination code largely copied from the issues component; not tested
since we can't currently add multiple screenshots in a FB submission yet
- Screenshot display in details isn't scaling properly -- if a
screenshot comes in that's extra wide or extra long (e.g. more than
`360px` in either dimension, it'll get cropped to fit within `360px` x
`360px`. However the design specifies that the entire image should
instead scale down to fit within this box, rather than getting cropped
- When clicking into full screen/zoom-in mode, the modal should take up
the whole screen rather than staying the same dimensions as the image.
(see designs for reference)
  • Loading branch information
michellewzhang authored Apr 1, 2024
1 parent 6ddc371 commit 4a30659
Show file tree
Hide file tree
Showing 5 changed files with 405 additions and 35 deletions.
9 changes: 0 additions & 9 deletions static/app/components/feedback/feedbackItem/feedbackItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import FeedbackItemHeader from 'sentry/components/feedback/feedbackItem/feedback
import Section from 'sentry/components/feedback/feedbackItem/feedbackItemSection';
import FeedbackReplay from 'sentry/components/feedback/feedbackItem/feedbackReplay';
import MessageSection from 'sentry/components/feedback/feedbackItem/messageSection';
import {ScreenshotSection} from 'sentry/components/feedback/feedbackItem/screenshotSection';
import TagsSection from 'sentry/components/feedback/feedbackItem/tagsSection';
import PanelItem from 'sentry/components/panels/panelItem';
import QuestionTooltip from 'sentry/components/questionTooltip';
Expand Down Expand Up @@ -49,14 +48,6 @@ export default function FeedbackItem({feedbackItem, eventData, tags}: Props) {
<MessageSection eventData={eventData} feedbackItem={feedbackItem} />
</Section>

{eventData && (
<ScreenshotSection
event={eventData}
organization={organization}
projectSlug={feedbackItem.project.slug}
/>
)}

{!crashReportId || (crashReportId && url) ? (
<Section icon={<IconLink size="xs" />} title={t('URL')}>
<TextCopyInput size="sm">
Expand Down
172 changes: 172 additions & 0 deletions static/app/components/feedback/feedbackItem/feedbackScreenshot.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
import type {ReactEventHandler} from 'react';
import {Fragment, 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';

type Props = {
eventId: Event['id'];
openVisualizationModal: (eventAttachment: EventAttachment) => void;
organization: Organization;
projectSlug: Project['slug'];
screenshot: EventAttachment;
screenshotInFocus: number;
totalScreenshots: number;
onNext?: ReactEventHandler;
onPrevious?: ReactEventHandler;
};

function FeedbackScreenshot({
eventId,
organization,
screenshot,
screenshotInFocus,
onNext,
onPrevious,
totalScreenshots,
projectSlug,
openVisualizationModal,
}: 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>
);
}

return (
<Role organization={organization} role={organization.attachmentsRole}>
{({hasRole}) => {
if (!hasRole) {
return null;
}
return <StyledPanel>{renderContent(screenshot)}</StyledPanel>;
}}
</Role>
);
}

export default FeedbackScreenshot;

const StyledPanel = styled(Panel)`
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
margin-bottom: 0;
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: 1px solid ${p => p.theme.border};
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};
`}
`;

const StyledLoadingIndicator = styled('div')`
position: absolute;
display: flex;
align-items: center;
justify-content: center;
height: 100%;
`;

const StyledImageWrapper = styled('button')`
cursor: zoom-in;
background: none;
padding: 0;
border-radius: ${p => p.theme.borderRadius};
border: 0;
overflow: hidden;
`;

const StyledImageVisualization = styled(ImageVisualization)`
z-index: 1;
border: 0;
img {
width: auto;
height: auto;
}
`;
16 changes: 15 additions & 1 deletion static/app/components/feedback/feedbackItem/messageSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,22 @@ import styled from '@emotion/styled';
import FeedbackItemUsername from 'sentry/components/feedback/feedbackItem/feedbackItemUsername';
import FeedbackTimestampsTooltip from 'sentry/components/feedback/feedbackItem/feedbackTimestampsTooltip';
import FeedbackViewers from 'sentry/components/feedback/feedbackItem/feedbackViewers';
import {ScreenshotSection} from 'sentry/components/feedback/feedbackItem/screenshotSection';
import {Flex} from 'sentry/components/profiling/flex';
import TimeSince from 'sentry/components/timeSince';
import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import type {Event} from 'sentry/types';
import type {FeedbackIssue} from 'sentry/utils/feedback/types';
import useOrganization from 'sentry/utils/useOrganization';

interface Props {
eventData: Event | undefined;
feedbackItem: FeedbackIssue;
}

export default function MessageSection({eventData, feedbackItem}: Props) {
const organization = useOrganization();
return (
<Fragment>
<Flex wrap="wrap" flex="1 1 auto" gap={space(1)} justify="space-between">
Expand All @@ -34,6 +37,13 @@ export default function MessageSection({eventData, feedbackItem}: Props) {
</Flex>
<Blockquote>
<pre>{feedbackItem.metadata.message}</pre>
{eventData && (
<ScreenshotSection
event={eventData}
organization={organization}
projectSlug={feedbackItem.project.slug}
/>
)}
</Blockquote>
<Flex justify="flex-end">
<Flex gap={space(1)} align="center">
Expand Down Expand Up @@ -61,11 +71,15 @@ const Blockquote = styled('blockquote')`
margin: 0;
background: ${p => p.theme.purple100};
display: flex;
flex-direction: column;
gap: ${space(2)};
border-left: 2px solid ${p => p.theme.purple300};
padding: ${space(2)};
& > pre {
margin: 0;
margin-bottom: 0;
background: none;
font-family: inherit;
font-size: ${p => p.theme.fontSizeMedium};
Expand Down
Loading

0 comments on commit 4a30659

Please sign in to comment.