Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(feedback): Update user feedback screenshot and cropping to align with designs #11227

Merged
merged 23 commits into from
Mar 26, 2024
Merged

feat(feedback): Update user feedback screenshot and cropping to align with designs #11227

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
de4562c
customize crop color, BES naming & padding
c298lee Mar 21, 2024
e441bc8
fix BEM naming
c298lee Mar 21, 2024
66565b6
feat(opentelemetry): Do not capture exceptions for timed events (#11221)
lforst Mar 21, 2024
d87efcf
fix(metrics): use web-vitals ttfb calculation (#11185)
AbhiPrasad Mar 21, 2024
77ef529
fix(node): Clear ANR timer on stop (#11229)
timfish Mar 21, 2024
e0e0c17
fix(feedback): Fix screenshot black bars in Safari (#11233)
c298lee Mar 21, 2024
6c770ad
meta: CHANGELOG for 8.0.0-alpha.5
AbhiPrasad Mar 22, 2024
5a2e064
Apply suggestions from code review
Lms24 Mar 22, 2024
829ab27
release: 8.0.0-alpha.5
getsentry-bot Mar 22, 2024
ecb82fb
build(bundles): Use ES2018 for bundles (#11201)
s1gr1d Mar 22, 2024
f6149d8
fix(sveltekit): Ensure sub requests are recorded as child spans of pa…
Lms24 Mar 22, 2024
02f1ba6
fix(node): Local variables skipped after Promise (#11234)
timfish Mar 22, 2024
28d5ebe
ref(replay): Move `@sentry/replay` code to `@sentry-internal/replay` …
s1gr1d Mar 22, 2024
5dd7202
feat(tracing): Remove `instrumentRoutingWithDefaults` (#11240)
AbhiPrasad Mar 22, 2024
1e0c078
ref(v8): change integration.setupOnce signature (#11238)
AbhiPrasad Mar 22, 2024
d5d661e
chore: Add angular note to 8.0.0-alpha.5 changelog (#11254)
AbhiPrasad Mar 22, 2024
856766a
crop looks more like photoshop
c298lee Mar 22, 2024
8566ce8
Merge branch 'develop' into cl/screenshot-styles
c298lee Mar 22, 2024
1c11f74
Merge branch 'develop' into cl/screenshot-styles
c298lee Mar 26, 2024
1167d02
Merge branch 'develop' into cl/screenshot-styles
c298lee Mar 26, 2024
8c2bc3a
move crop corner border styles to constants
c298lee Mar 26, 2024
85522f6
PR comment fix
c298lee Mar 26, 2024
7b41ae0
lint
c298lee Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 41 additions & 37 deletions packages/feedback/src/screenshot/components/ScreenshotEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { useTakeScreenshot } from './useTakeScreenshot';
const CROP_BUTTON_SIZE = 30;
const CROP_BUTTON_BORDER = 3;
const CROP_BUTTON_OFFSET = CROP_BUTTON_SIZE + CROP_BUTTON_BORDER;
const DPI = WINDOW.devicePixelRatio;

interface FactoryParams {
h: typeof hType;
Expand Down Expand Up @@ -79,8 +80,14 @@ export function makeScreenshotEditorComponent({ h, imageBuffer, dialog }: Factor
const cropper = croppingRef.current;
const imageDimensions = constructRect(getContainedSize(imageBuffer));
if (cropper) {
cropper.width = imageDimensions.width;
cropper.height = imageDimensions.height;
cropper.width = imageDimensions.width * DPI;
cropper.height = imageDimensions.height * DPI;
cropper.style.width = `${imageDimensions.width}px`;
cropper.style.height = `${imageDimensions.height}px`;
const ctx = cropper.getContext('2d');
if (ctx) {
ctx.scale(DPI, DPI);
}
Comment on lines +83 to +90
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reduces the crop rectangle blurriness, which is a known issue on retina screens: https://developer.mozilla.org/en-US/docs/Web/API/Window/devicePixelRatio

}

const cropButton = cropContainerRef.current;
Expand All @@ -104,9 +111,9 @@ export function makeScreenshotEditorComponent({ h, imageBuffer, dialog }: Factor
if (!ctx) {
return;
}

const imageDimensions = constructRect(getContainedSize(imageBuffer));
const croppingBox = constructRect(croppingRect);

ctx.clearRect(0, 0, imageDimensions.width, imageDimensions.height);

// draw gray overlay around the selection
Expand All @@ -115,9 +122,12 @@ export function makeScreenshotEditorComponent({ h, imageBuffer, dialog }: Factor
ctx.clearRect(croppingBox.x, croppingBox.y, croppingBox.width, croppingBox.height);

// draw selection border
ctx.strokeStyle = 'purple';
ctx.strokeStyle = '#FFFFFF';
ctx.lineWidth = 3;
ctx.strokeRect(croppingBox.x, croppingBox.y, croppingBox.width, croppingBox.height);
ctx.strokeRect(croppingBox.x + 1, croppingBox.y + 1, croppingBox.width - 2, croppingBox.height - 2);
ctx.strokeStyle = '#000000';
ctx.lineWidth = 1;
ctx.strokeRect(croppingBox.x + 3, croppingBox.y + 3, croppingBox.width - 6, croppingBox.height - 6);
}, [croppingRect]);

function onGrabButton(e: Event, corner: string): void {
Expand All @@ -143,32 +153,32 @@ export function makeScreenshotEditorComponent({ h, imageBuffer, dialog }: Factor
const mouseX = e.clientX - cropBoundingRect.x;
const mouseY = e.clientY - cropBoundingRect.y;
switch (corner) {
case 'topleft':
case 'top-left':
setCroppingRect(prev => ({
...prev,
startX: Math.min(Math.max(0, mouseX), prev.endX - CROP_BUTTON_OFFSET),
startY: Math.min(Math.max(0, mouseY), prev.endY - CROP_BUTTON_OFFSET),
}));
break;
case 'topright':
case 'top-right':
setCroppingRect(prev => ({
...prev,
endX: Math.max(Math.min(mouseX, cropCanvas.width), prev.startX + CROP_BUTTON_OFFSET),
endX: Math.max(Math.min(mouseX, cropCanvas.width / DPI), prev.startX + CROP_BUTTON_OFFSET),
startY: Math.min(Math.max(0, mouseY), prev.endY - CROP_BUTTON_OFFSET),
}));
break;
case 'bottomleft':
case 'bottom-left':
setCroppingRect(prev => ({
...prev,
startX: Math.min(Math.max(0, mouseX), prev.endX - CROP_BUTTON_OFFSET),
endY: Math.max(Math.min(mouseY, cropCanvas.height), prev.startY + CROP_BUTTON_OFFSET),
endY: Math.max(Math.min(mouseY, cropCanvas.height / DPI), prev.startY + CROP_BUTTON_OFFSET),
}));
break;
case 'bottomright':
case 'bottom-right':
setCroppingRect(prev => ({
...prev,
endX: Math.max(Math.min(mouseX, cropCanvas.width), prev.startX + CROP_BUTTON_OFFSET),
endY: Math.max(Math.min(mouseY, cropCanvas.height), prev.startY + CROP_BUTTON_OFFSET),
endX: Math.max(Math.min(mouseX, cropCanvas.width / DPI), prev.startX + CROP_BUTTON_OFFSET),
endY: Math.max(Math.min(mouseY, cropCanvas.height / DPI), prev.startY + CROP_BUTTON_OFFSET),
}));
break;
}
Expand Down Expand Up @@ -238,40 +248,40 @@ export function makeScreenshotEditorComponent({ h, imageBuffer, dialog }: Factor
return (
<div class="editor">
<style dangerouslySetInnerHTML={styles} />
<div class="canvasContainer" ref={canvasContainerRef}>
<div class="cropButtonContainer" style={{ position: 'absolute' }} ref={cropContainerRef}>
<div class="editor__canvas-container" ref={canvasContainerRef}>
<div class="editor__crop-container" style={{ position: 'absolute' }} ref={cropContainerRef}>
<canvas style={{ position: 'absolute' }} ref={croppingRef}></canvas>
<CropCorner
left={croppingRect.startX}
top={croppingRect.startY}
left={croppingRect.startX - CROP_BUTTON_BORDER}
top={croppingRect.startY - CROP_BUTTON_BORDER}
onGrabButton={onGrabButton}
corner="topleft"
corner="top-left"
></CropCorner>
<CropCorner
left={croppingRect.endX - CROP_BUTTON_SIZE}
top={croppingRect.startY}
left={croppingRect.endX - CROP_BUTTON_SIZE + CROP_BUTTON_BORDER}
top={croppingRect.startY - CROP_BUTTON_BORDER}
onGrabButton={onGrabButton}
corner="topright"
corner="top-right"
></CropCorner>
<CropCorner
left={croppingRect.startX}
top={croppingRect.endY - CROP_BUTTON_SIZE}
left={croppingRect.startX - CROP_BUTTON_BORDER}
top={croppingRect.endY - CROP_BUTTON_SIZE + CROP_BUTTON_BORDER}
onGrabButton={onGrabButton}
corner="bottomleft"
corner="bottom-left"
></CropCorner>
<CropCorner
left={croppingRect.endX - CROP_BUTTON_SIZE}
top={croppingRect.endY - CROP_BUTTON_SIZE}
left={croppingRect.endX - CROP_BUTTON_SIZE + CROP_BUTTON_BORDER}
top={croppingRect.endY - CROP_BUTTON_SIZE + CROP_BUTTON_BORDER}
onGrabButton={onGrabButton}
corner="bottomright"
corner="bottom-right"
></CropCorner>
<div
style={{
left: Math.max(0, croppingRect.endX - 191),
top: Math.max(0, croppingRect.endY + 8),
display: confirmCrop ? 'flex' : 'none',
}}
class="crop-btn-group"
class="editor__crop-btn-group"
>
<button
onClick={e => {
Expand All @@ -280,8 +290,8 @@ export function makeScreenshotEditorComponent({ h, imageBuffer, dialog }: Factor
setCroppingRect({
startX: 0,
startY: 0,
endX: croppingRef.current.width,
endY: croppingRef.current.height,
endX: croppingRef.current.width / DPI,
endY: croppingRef.current.height / DPI,
});
}
setConfirmCrop(false);
Expand Down Expand Up @@ -321,16 +331,10 @@ function CropCorner({
}): VNode {
return (
<button
class="crop-btn"
class={`editor__crop-corner editor__crop-corner--${corner} `}
style={{
top: top,
left: left,
borderTop: corner === 'topleft' || corner === 'topright' ? 'solid purple' : 'none',
borderLeft: corner === 'topleft' || corner === 'bottomleft' ? 'solid purple' : 'none',
borderRight: corner === 'topright' || corner === 'bottomright' ? 'solid purple' : 'none',
borderBottom: corner === 'bottomleft' || corner === 'bottomright' ? 'solid purple' : 'none',
borderWidth: `${CROP_BUTTON_BORDER}px`,
cursor: corner === 'topleft' || corner === 'bottomright' ? 'nwse-resize' : 'nesw-resize',
}}
onMouseDown={e => {
e.preventDefault();
Expand Down
43 changes: 32 additions & 11 deletions packages/feedback/src/screenshot/components/ScreenshotInput.css.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { DOCUMENT } from '../../constants';
export function createScreenshotInputStyles(): HTMLStyleElement {
const style = DOCUMENT.createElement('style');

const surface200 = '#FAF9FB';
const gray100 = '#F0ECF3';
const surface200 = '#1A141F';
const gray100 = '#302735';

style.textContent = `
.dialog__content:has(.editor) {
Expand All @@ -16,6 +16,9 @@ export function createScreenshotInputStyles(): HTMLStyleElement {
}

.editor {
padding: 10px;
padding-top: 65px;
padding-bottom: 65px;
flex-grow: 1;

background-color: ${surface200};
Expand All @@ -35,24 +38,19 @@ export function createScreenshotInputStyles(): HTMLStyleElement {
);
}

.canvasContainer {
.editor__canvas-container {
width: 100%;
height: 100%;
position: relative;
}

.canvasContainer canvas {
.editor__canvas-container canvas {
width: 100%;
height: 100%;
object-fit: contain;
}

.cropper {
width: 100%;
height: 100%;
}

.crop-btn-group {
.editor__crop-btn-group {
padding: 8px;
gap: 8px;
border-radius: var(--form-content-border-radius);
Expand All @@ -61,11 +59,34 @@ export function createScreenshotInputStyles(): HTMLStyleElement {
position: absolute;
}

.crop-btn {
.editor__crop-corner {
width: 30px;
height: 30px;
position: absolute;
background: none;
border: solid #FFFFFF;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add this to constants and share it with the canvas style?

border-width: 3px;
}

.editor__crop-corner--top-left {
cursor: nwse-resize;
border-right: none;
border-bottom: none;
}
.editor__crop-corner--top-right {
cursor: nesw-resize;
border-left: none;
border-bottom: none;
}
.editor__crop-corner--bottom-left {
cursor: nesw-resize;
border-right: none;
border-top: none;
}
.editor__crop-corner--bottom-right {
cursor: nwse-resize;
border-left: none;
border-top: none;
}
`;

Expand Down