fix(react): address Doctor findings (#1143)

Accessibility/semantics pass aligned with React Doctor: role-based spans/divs become real buttons, role=status becomes <output>, modals use native <dialog>, author/peer flags render as <img>, and embed/challenge iframes are sandboxed. Includes review follow-ups: correct button chrome/centering/themed dialog colors and focus rings, the missing aria-label i18n keys (all languages), the Yandex image-search URL, clearer .bso setup steps, a keyboard-accessible image-search dropdown, and removal of the post-edit ('comment edited') display (mod edits unchanged).
This commit is contained in:
Tommaso Casaburi
2026-05-29 17:09:07 +07:00
committed by GitHub
parent ac95e58433
commit e082c7b428
132 changed files with 1753 additions and 1224 deletions
@@ -174,7 +174,7 @@ describe('CommentMedia', () => {
expect(container.querySelector('img[src="assets/filedeleted-res.gif"]')).toBeTruthy();
expect(onMediaLoadFailureChange).toHaveBeenCalledWith('https://files.catbox.moe/missing.jpg');
const status = container.querySelector('[role="status"]');
const status = container.querySelector('output');
expect(status?.textContent).toContain('media_failed_to_load_inline:files.catbox.moe');
expect(status?.textContent).toContain('media_failed_to_load_open_source');
expect(status?.textContent).not.toContain('media_failed_to_load_hint');
@@ -209,7 +209,7 @@ describe('CommentMedia', () => {
image?.dispatchEvent(new Event('error', { bubbles: true }));
});
const status = container.querySelector('[role="status"]');
const status = container.querySelector('output');
expect(status?.textContent).toBe('');
expect(status?.getAttribute('aria-label')).toBe(
'media_failed_to_load. media_failed_to_load_source:files.catbox.moe. media_failed_to_load_hint. media_failed_to_load_open_source.',
@@ -273,7 +273,7 @@ describe('CommentMedia', () => {
expect(container.textContent).toContain('example.com');
const fallbackButton = Array.from(container.querySelectorAll('span')).find((node) => node.textContent === 'example.com' && node.getAttribute('role') === 'button');
const fallbackButton = Array.from(container.querySelectorAll('button')).find((node) => node.textContent === 'example.com');
await act(async () => {
fallbackButton?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
});
@@ -296,7 +296,7 @@ describe('CommentMedia', () => {
expect(container.querySelector('[data-testid="embed"]')?.textContent).toContain('https://youtu.be/test');
const closeButton = Array.from(container.querySelectorAll('span')).find((node) => node.textContent === 'close');
const closeButton = Array.from(container.querySelectorAll('button')).find((node) => node.textContent === 'close');
expect(closeButton).toBeTruthy();
await act(async () => {
@@ -55,14 +55,31 @@
}
.thumbnailSmall span,
.thumbnailSmall a {
.thumbnailSmall a,
.thumbnailSmall .noThumbnailButton {
width: 100%;
display: inline-block;
word-wrap: break-word;
overflow-wrap: break-word;
text-align: center;
cursor: pointer;
}
.noThumbnailButton {
appearance: none;
background: transparent;
border: 0;
color: inherit;
cursor: pointer;
font: inherit;
margin: 0;
padding: 0;
}
.noThumbnailButton:focus-visible {
outline: 1px dotted var(--post-link-text-color);
outline-offset: 1px;
}
.thumbnail {
float: left;
}
@@ -75,6 +92,33 @@
cursor: pointer;
}
.mediaToggleButton {
all: unset;
cursor: pointer;
display: inline-flex;
align-items: center;
justify-content: center;
max-width: 100%;
max-height: 100%;
line-height: 0;
}
.mediaToggleButton:focus-visible {
outline: 1px dotted var(--post-link-text-color);
outline-offset: 1px;
}
.mediaToggleButton img,
.mediaToggleButton video {
display: block;
}
.thumbnailBig > .mediaToggleButton,
.thumbnailSmall > .mediaToggleButton {
width: 100%;
height: 100%;
}
.hide {
display: none !important;
}
+67 -178
View File
@@ -50,13 +50,13 @@ const MediaLoadFailure = ({ compact = false, url }: { compact?: boolean; url?: s
return (
<>
<img className={styles.fileDeleted} src='assets/filedeleted-res.gif' alt='' aria-hidden='true' />
<span className={styles.mediaLoadFailureStatus} role='status' aria-label={statusLabel} title={statusLabel} />
<output className={styles.mediaLoadFailureStatus} aria-label={statusLabel} title={statusLabel} />
</>
);
}
return (
<span className={styles.mediaLoadFailure} role='status' aria-label={statusLabel} title={statusLabel}>
<output className={styles.mediaLoadFailure} aria-label={statusLabel} title={statusLabel}>
<img className={styles.fileDeleted} src='assets/filedeleted-res.gif' alt='' aria-hidden='true' />
<span className={styles.mediaLoadFailureSource}>{inline}</span>
{url && (
@@ -64,7 +64,7 @@ const MediaLoadFailure = ({ compact = false, url }: { compact?: boolean; url?: s
{openSourceLabel}
</a>
)}
</span>
</output>
);
};
@@ -109,92 +109,40 @@ const Thumbnail = ({
const iframeThumbnail = patternThumbnailUrl || thumbnail;
const { frameUrl: gifFrameUrl, status: gifFrameStatus } = gifFrameState;
const hasThumbnail = getHasThumbnail(commentMediaInfo, url);
const gifThumbnailButtonProps =
gifFrameStatus === 'loading'
? {
role: 'button' as const,
tabIndex: 0,
onKeyDown: (e: React.KeyboardEvent<HTMLSpanElement>) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setShowThumbnail(false);
}
},
onClick: () => setShowThumbnail(false),
}
: {};
const handleOpenMedia = () => setShowThumbnail(false);
if (type === 'gif') {
thumbnailComponent =
gifFrameStatus === 'loading' ? (
<span className={styles.gifPlaceholder} aria-label='Loading GIF thumbnail' {...gifThumbnailButtonProps} />
<button type='button' className={`${styles.gifPlaceholder} ${styles.mediaToggleButton}`} aria-label='Loading GIF thumbnail' onClick={handleOpenMedia} />
) : (
<img
src={gifFrameUrl || url}
alt=''
role='button'
tabIndex={0}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setShowThumbnail(false);
}
}}
onClick={() => setShowThumbnail(false)}
/>
<button type='button' className={styles.mediaToggleButton} aria-label='Open GIF' onClick={handleOpenMedia}>
<img src={gifFrameUrl || url} alt='' />
</button>
);
} else if (type === 'video') {
thumbnailComponent = thumbnail ? (
<img src={thumbnail} alt='' />
<img src={thumbnail} alt='Video thumbnail' />
) : (
// show first frame of the video, as a workaround for Safari not loading thumbnails
<video
src={`${url}#t=0.001`}
role='button'
tabIndex={0}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setShowThumbnail(false);
}
}}
onClick={() => setShowThumbnail(false)}
/>
<button type='button' className={styles.mediaToggleButton} aria-label='Open video' onClick={handleOpenMedia}>
<video src={`${url}#t=0.001`} aria-label='Video thumbnail' />
</button>
);
} else if (type === 'webpage') {
thumbnailComponent = (
<img
src={thumbnail}
alt=''
role='button'
tabIndex={0}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setShowThumbnail(false);
}
}}
onClick={() => setShowThumbnail(false)}
/>
<button type='button' className={styles.mediaToggleButton} aria-label='Open webpage preview' onClick={handleOpenMedia}>
<img src={thumbnail} alt='' />
</button>
);
} else if (type === 'iframe') {
thumbnailComponent = iframeThumbnail ? (
<img
src={iframeThumbnail}
alt=''
role='button'
tabIndex={0}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setShowThumbnail(false);
}
}}
onClick={() => setShowThumbnail(false)}
/>
<button type='button' className={styles.mediaToggleButton} aria-label='Open embedded media preview' onClick={handleOpenMedia}>
<img src={iframeThumbnail} alt='' />
</button>
) : null;
} else if (type === 'audio') {
thumbnailComponent = <audio src={url} controls />;
thumbnailComponent = <audio src={url} aria-label='Audio preview' controls />;
}
const thumbnailSmallPadding = isMobile ? styles.thumbnailMobile : styles.thumbnailReplyDesktop;
@@ -204,8 +152,9 @@ const Thumbnail = ({
const noThumbnailLink =
!hasThumbnail && linkWithoutThumbnail ? (
canEmbed(linkWithoutThumbnail) ? (
<span
role='button'
<button
type='button'
className={styles.noThumbnailButton}
tabIndex={0}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
@@ -216,7 +165,7 @@ const Thumbnail = ({
onClick={() => setShowThumbnail(false)}
>
{fallbackLinkLabel}
</span>
</button>
) : (
<a href={url} target='_blank' rel='noopener noreferrer'>
{fallbackLinkLabel}
@@ -227,20 +176,9 @@ const Thumbnail = ({
return deleted || removed || purged ? (
<img className={styles.fileDeleted} src='assets/filedeleted-res.gif' alt='File deleted' />
) : spoiler ? (
<img
className={styles.spoiler}
src='assets/spoiler.png'
alt=''
role='button'
tabIndex={0}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setShowThumbnail(false);
}
}}
onClick={() => setShowThumbnail(false)}
/>
<button type='button' className={styles.mediaToggleButton} aria-label='Open spoiler media' onClick={handleOpenMedia}>
<img className={styles.spoiler} src='assets/spoiler.png' alt='' />
</button>
) : isOutOfFeed ? (
<span className={`${isFloatingEmbed ? styles.floatingEmbed : styles.communityAvatar}`}>{thumbnailComponent}</span>
) : isMobile || isReply ? (
@@ -272,43 +210,23 @@ const Media = ({ commentMediaInfo, disableToggle, isReply, setShowThumbnail }: M
{type === 'iframe' && url ? (
<Embed url={url} />
) : type === 'gif' ? (
<img
src={url}
alt=''
role={disableToggle ? undefined : 'button'}
tabIndex={disableToggle ? undefined : 0}
onKeyDown={
disableToggle
? undefined
: (e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setShowThumbnail(true);
}
}
}
onClick={disableToggle ? undefined : () => setShowThumbnail(true)}
/>
disableToggle ? (
<img src={url} alt='' />
) : (
<button type='button' className={styles.mediaToggleButton} aria-label='Collapse GIF' onClick={() => setShowThumbnail(true)}>
<img src={url} alt='' />
</button>
)
) : type === 'video' ? (
<video src={url} controls autoPlay loop muted={!unmuteExpandedVideoSound} />
<video src={url} aria-label={t('video')} controls autoPlay loop muted={!unmuteExpandedVideoSound} />
) : type === 'webpage' ? (
<img
src={thumbnail}
alt=''
role={disableToggle ? undefined : 'button'}
tabIndex={disableToggle ? undefined : 0}
onKeyDown={
disableToggle
? undefined
: (e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setShowThumbnail(true);
}
}
}
onClick={disableToggle ? undefined : () => setShowThumbnail(true)}
/>
disableToggle ? (
<img src={thumbnail} alt='' />
) : (
<button type='button' className={styles.mediaToggleButton} aria-label='Collapse webpage preview' onClick={() => setShowThumbnail(true)}>
<img src={thumbnail} alt='' />
</button>
)
) : null}
{isMobile && type && (
<div className={styles.fileInfo}>
@@ -321,9 +239,9 @@ const Media = ({ commentMediaInfo, disableToggle, isReply, setShowThumbnail }: M
)}
{isMobile && (type === 'iframe' || type === 'video' || type === 'audio') && (
<div className={styles.closeButton}>
<span
<button
type='button'
className='button'
role='button'
tabIndex={0}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
@@ -334,7 +252,7 @@ const Media = ({ commentMediaInfo, disableToggle, isReply, setShowThumbnail }: M
onClick={() => setShowThumbnail(true)}
>
{t('close')}
</span>
</button>
</div>
)}
</span>
@@ -397,20 +315,9 @@ const Image = ({
className={`${isOutOfFeed ? styles.communityAvatar : styles.thumbnailBig} ${styles.thumbnail} ${isImageExpanded && isMobile ? styles.removeFloat : ''}`}
style={spoilerDimensions}
>
<img
className={styles.spoiler}
src='assets/spoiler.png'
alt=''
role='button'
tabIndex={0}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setIsImageExpanded(true);
}
}}
onClick={() => setIsImageExpanded(true)}
/>
<button type='button' className={styles.mediaToggleButton} aria-label='Open spoiler media' onClick={() => setIsImageExpanded(true)}>
<img className={styles.spoiler} src='assets/spoiler.png' alt='' />
</button>
</span>
);
}
@@ -424,26 +331,17 @@ const Image = ({
>
{hasError ? (
<MediaLoadFailure compact url={url} />
) : disableToggle ? (
<img src={url} onError={handleError} onLoad={handleLoad} alt='' />
) : (
<img
src={url}
onError={handleError}
onLoad={handleLoad}
alt=''
role={disableToggle ? undefined : 'button'}
tabIndex={disableToggle ? undefined : 0}
onKeyDown={
disableToggle
? undefined
: (e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setIsImageExpanded(!isImageExpanded);
}
}
}
onClick={disableToggle ? undefined : () => setIsImageExpanded(!isImageExpanded)}
/>
<button
type='button'
className={styles.mediaToggleButton}
aria-label={isImageExpanded ? 'Collapse image' : 'Expand image'}
onClick={() => setIsImageExpanded(!isImageExpanded)}
>
<img src={url} onError={handleError} onLoad={handleLoad} alt='' />
</button>
)}
</span>
{isImageExpanded && type && (
@@ -465,26 +363,17 @@ const Image = ({
>
{hasError ? (
<MediaLoadFailure url={url} />
) : disableToggle ? (
<img src={url} onError={handleError} onLoad={handleLoad} alt='' />
) : (
<img
src={url}
onError={handleError}
onLoad={handleLoad}
alt=''
role='button'
tabIndex={0}
onKeyDown={
disableToggle
? undefined
: (e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setIsImageExpanded(!isImageExpanded);
}
}
}
onClick={disableToggle ? undefined : () => setIsImageExpanded(!isImageExpanded)}
/>
<button
type='button'
className={styles.mediaToggleButton}
aria-label={isImageExpanded ? 'Collapse image' : 'Expand image'}
onClick={() => setIsImageExpanded(!isImageExpanded)}
>
<img src={url} onError={handleError} onLoad={handleLoad} alt='' />
</button>
)}
</span>
);