-
Notifications
You must be signed in to change notification settings - Fork 305
feat: Allow users to pause gifs #19909
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.
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
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -48,6 +48,7 @@ interface BaseImageProps extends React.HTMLProps<HTMLDivElement> { | |||||
| interface RemoteDataImageProps extends BaseImageProps { | ||||||
| image: AssetRemoteData; | ||||||
| imageSizes?: {width: string; height: string; ratio: number}; | ||||||
| fileType?: string; | ||||||
| } | ||||||
| interface AssetImageProps extends BaseImageProps { | ||||||
| image: MediumImage; | ||||||
|
|
@@ -56,12 +57,13 @@ interface AssetImageProps extends BaseImageProps { | |||||
| export const AssetImage = ({image, alt, ...props}: AssetImageProps) => { | ||||||
| const {resource} = useKoSubscribableChildren(image, ['resource']); | ||||||
|
|
||||||
| return <Image image={resource} imageSizes={image} alt={alt} {...props} />; | ||||||
| return <Image image={resource} imageSizes={image} fileType={image.file_type} alt={alt} {...props} />; | ||||||
| }; | ||||||
|
|
||||||
| export const Image = ({ | ||||||
| image, | ||||||
| imageSizes, | ||||||
| fileType, | ||||||
| onClick, | ||||||
| className, | ||||||
| isQuote = false, | ||||||
|
|
@@ -88,7 +90,7 @@ export const Image = ({ | |||||
| 'application/octet-stream', // Octet-stream is required to paste images from clipboard | ||||||
| ...Config.getConfig().ALLOWED_IMAGE_TYPES, | ||||||
| ]; | ||||||
| const url = await getAssetUrl(image, allowedImageTypes); | ||||||
| const url = await getAssetUrl(image, allowedImageTypes, fileType); | ||||||
| if (isUnmouted.current) { | ||||||
| // Avoid re-rendering a component that is umounted | ||||||
| return; | ||||||
|
|
@@ -115,27 +117,40 @@ export const Image = ({ | |||||
| const dummyImageUrl = `data:image/svg+xml;utf8,<svg aria-hidden="true" xmlns='http://www.w3.org/2000/svg' viewBox='0 0 1 1' width='${imageSizes?.width}' height='${imageSizes?.height}'></svg>`; | ||||||
| const assetUrl = imageUrl?.url || dummyImageUrl; | ||||||
| const isLoading = !imageUrl; | ||||||
| const ImageElement = ({src}: {src: string}) => ( | ||||||
| <img | ||||||
| css={{...getImageStyle(imageSizes), ...imageStyles}} | ||||||
| src={src} | ||||||
| role="presentation" | ||||||
| alt={alt} | ||||||
| data-uie-name={isLoading ? 'image-loader' : 'image-asset-img'} | ||||||
| onClick={event => { | ||||||
| if (!isLoading) { | ||||||
| onClick?.(event); | ||||||
| } | ||||||
| }} | ||||||
| /> | ||||||
| ); | ||||||
|
|
||||||
| return ( | ||||||
| <InViewport | ||||||
| onVisible={() => setIsInViewport(true)} | ||||||
| css={getWrapperStyles(!!onClick)} | ||||||
| className={cx(className, {'loading-dots image-asset--no-image': isLoading})} | ||||||
| onClick={event => { | ||||||
| if (!isLoading) { | ||||||
| onClick?.(event); | ||||||
| } | ||||||
| }} | ||||||
| data-uie-status={isLoading ? 'loading' : 'loaded'} | ||||||
| {...props} | ||||||
| > | ||||||
| <img | ||||||
| css={{...getImageStyle(imageSizes), ...imageStyles}} | ||||||
| src={assetUrl} | ||||||
| role="presentation" | ||||||
| alt={alt} | ||||||
| data-uie-name={isLoading ? 'image-loader' : 'image-asset-img'} | ||||||
| /> | ||||||
| {imageUrl?.pauseFrameUrl ? ( | ||||||
| <div className="image-asset--animated-wrapper"> | ||||||
| <ImageElement src={imageUrl?.pauseFrameUrl} /> | ||||||
| <details open className="image-asset--animated"> | ||||||
|
||||||
| <details open className="image-asset--animated"> | |
| <details className="image-asset--animated"> |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the HTML details/summary elements for a play/pause toggle is semantically incorrect. These elements are designed for expandable/collapsible content disclosure, not for media controls. Consider using a standard button element with proper ARIA attributes like aria-pressed to indicate the play/pause state, which would better convey the control's purpose to assistive technologies.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |
|
|
||
| import {useEffect, useState} from 'react'; | ||
|
|
||
| import {GifUtil} from 'gifwrap'; | ||
|
||
| import {container} from 'tsyringe'; | ||
|
|
||
| import {AssetRemoteData} from 'Repositories/assets/AssetRemoteData'; | ||
|
|
@@ -30,6 +31,7 @@ import {useKoSubscribableChildren} from 'Util/ComponentUtil'; | |
|
|
||
| export type AssetUrl = { | ||
| url: string; | ||
| pauseFrameUrl?: string; | ||
| dispose: () => void; | ||
| }; | ||
|
|
||
|
|
@@ -59,7 +61,11 @@ export const useAssetTransfer = (message?: ContentMessage, assetRepository = con | |
| isPendingUpload: transferState === AssetTransferState.UPLOAD_PENDING, | ||
| isUploaded: transferState === AssetTransferState.UPLOADED, | ||
| isUploading: transferState === AssetTransferState.UPLOADING, | ||
| getAssetUrl: async (resource: AssetRemoteData, acceptedMimeTypes?: string[]): Promise<AssetUrl> => { | ||
| getAssetUrl: async ( | ||
| resource: AssetRemoteData, | ||
| acceptedMimeTypes?: string[], | ||
| fileType?: string, | ||
| ): Promise<AssetUrl> => { | ||
|
Comment on lines
+64
to
+68
|
||
| const blob = await assetRepository.load(resource); | ||
| if (!blob) { | ||
| throw new Error(`Asset could not be loaded`); | ||
|
|
@@ -68,9 +74,55 @@ export const useAssetTransfer = (message?: ContentMessage, assetRepository = con | |
| throw new Error(`Mime type not accepted "${blob.type}"`); | ||
| } | ||
| const url = URL.createObjectURL(blob); | ||
|
|
||
| const pauseFrameUrl = await (async () => { | ||
| if (blob.type === 'image/gif' || fileType === 'image/gif') { | ||
| try { | ||
| const gifArrayBuffer = await blob.arrayBuffer(); | ||
| const gifBuffer = Buffer.from(gifArrayBuffer); | ||
| const frame0 = await GifUtil.read(gifBuffer).then(gifFile => gifFile.frames[0]); | ||
| const canvas = document.createElement('canvas'); | ||
| const ctx = canvas.getContext('2d'); | ||
| if (!ctx) { | ||
| return undefined; | ||
| } | ||
| canvas.width = frame0.bitmap.width; | ||
| canvas.height = frame0.bitmap.height; | ||
|
|
||
| const imageData = ctx.createImageData(frame0.bitmap.width, frame0.bitmap.height); | ||
| imageData.data.set(frame0.bitmap.data); | ||
| ctx.putImageData(imageData, 0, 0); | ||
|
|
||
|
||
| const pauseImageBlob: Blob | null = await new Promise(resolve => { | ||
| canvas.toBlob( | ||
| blob => { | ||
| resolve(blob); | ||
| }, | ||
| 'image/jpeg', | ||
| 0.9, | ||
| ); | ||
| }); | ||
|
|
||
| if (pauseImageBlob) { | ||
| return URL.createObjectURL(pauseImageBlob); | ||
| } | ||
| return undefined; | ||
| } catch (_error) { | ||
| return undefined; | ||
| } | ||
| } | ||
| return undefined; | ||
| })(); | ||
|
|
||
| return { | ||
| dispose: () => URL.revokeObjectURL(url), | ||
| dispose: () => { | ||
| URL.revokeObjectURL(url); | ||
| if (pauseFrameUrl) { | ||
| URL.revokeObjectURL(pauseFrameUrl); | ||
| } | ||
| }, | ||
| url, | ||
| pauseFrameUrl, | ||
| }; | ||
| }, | ||
| transferState, | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -40,6 +40,7 @@ | |||||
| public readonly downloadProgress: ko.PureComputed<number | undefined>; | ||||||
| public readonly cancelDownload: () => void; | ||||||
| public file_size: number; | ||||||
| public readonly file_type: string; | ||||||
|
Check failure on line 43 in src/script/repositories/entity/message/FileAsset.ts
|
||||||
|
||||||
| public readonly file_type: string; | |
| public file_type: string; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -43,3 +43,58 @@ | |||||||||||||||||||||||||||||
| line-height: 1.3em; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| .image-asset--animated-wrapper { | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
| .image-asset--animated-wrapper { | |
| .image-asset--gif-controls-wrapper { |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The play/pause button uses a hardcoded color (#202020). This may not respect user theme preferences (light/dark mode) or brand color schemes. Consider using a CSS variable or theme token to ensure consistency across the application and proper contrast in different themes.
| border-color: transparent transparent transparent #202020; | |
| border-color: transparent transparent transparent var(--gif-play-button-color, #202020); |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The opacity value of 0.1 makes the play/pause button nearly invisible by default. This could make it difficult for users to discover this functionality. Consider using a higher opacity value (e.g., 0.5-0.7) or adding a subtle background to ensure the button is more discoverable while still being unobtrusive.
| opacity: 0.1; | |
| opacity: 0.6; |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The play/pause button lacks visible focus styling. Users navigating by keyboard won't have a visual indication when the button is focused, making it difficult to know where they are on the page. Add focus styles to the summary element.
| summary:focus, | |
| summary:focus-visible { | |
| outline: 2px solid #ffffff; | |
| outline-offset: 2px; | |
| } |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CSS selector [open] on line 79 is missing the element type qualifier. It should be details[open] to be more specific and match the pattern used elsewhere in the file (e.g., details summary on lines 85 and 89). This improves specificity and makes the selector's intent clearer.
| [open] summary { | |
| details[open] summary { |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The image uses absolute positioning which removes it from normal document flow. Both images (pause frame and animated) are absolutely positioned at the same location, which could cause layout issues or z-index conflicts. Ensure the positioning strategy doesn't break the layout in edge cases.
| .image-asset--animated img { | |
| position: absolute; | |
| top: 0; | |
| left: 0; | |
| display: inline-block; | |
| .image-asset--animated { | |
| position: relative; | |
| display: inline-block; | |
| } | |
| .image-asset--animated img { | |
| display: block; | |
| width: 100%; | |
| height: auto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable name contains a typo - 'isUnmouted' should be 'isUnmounted'. While this is a pre-existing typo, it reduces code readability and should be corrected.