Skip to content

Commit 639bfcb

Browse files
committed
fix: address CodeRabbit review — a11y, test coverage, and Tailwind typo
- SignInPromptModal: add focus save/restore, Escape key handler, tabIndex - SignInPromptModal.test.jsx: assert feature-specific copy in looped tests, add Escape/other-key tests (2 new tests, 1655 total) - InsightFloatingActionButton: fix ring-off-2 → ring-offset-2 typo
1 parent 234b60f commit 639bfcb

3 files changed

Lines changed: 52 additions & 2 deletions

File tree

src/components/InsightFloatingActionButton.jsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ export default function InsightFloatingActionButton({
6969
text-white rounded-full shadow-lg hover:shadow-xl
7070
items-center justify-center
7171
transition-all duration-200
72-
focus:outline-none focus:ring-2 focus:ring-amber-400 focus:ring-off-2
72+
focus:outline-none focus:ring-2 focus:ring-amber-400 focus:ring-offset-2
7373
touch-manipulation
7474
`}
7575
initial={{ scale: 0, opacity: 0 }}

src/components/SignInPromptModal.jsx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* See LICENSE for details.
55
*/
66

7+
import { useEffect, useRef } from 'react';
78
import { useTranslation } from 'react-i18next';
89
import { GoogleLogo } from '@phosphor-icons/react';
910
import { motion, AnimatePresence } from 'framer-motion';
@@ -13,20 +14,37 @@ function SignInPromptModal({ feature, isOpen, onClose }) {
1314
const { t } = useTranslation();
1415
const { signInWithGoogle } = useAuth();
1516
const featureLabel = t(`featureGate.${feature}`, { defaultValue: feature });
17+
const dialogRef = useRef(null);
18+
19+
useEffect(() => {
20+
if (!isOpen) return;
21+
22+
const prevFocus = document.activeElement;
23+
dialogRef.current?.querySelector('button')?.focus();
24+
25+
return () => {
26+
if (prevFocus && typeof prevFocus.focus === 'function') {
27+
prevFocus.focus();
28+
}
29+
};
30+
}, [isOpen]);
1631

1732
return (
1833
<AnimatePresence>
1934
{isOpen && (
2035
<motion.div
36+
ref={dialogRef}
2137
className="fixed inset-0 z-[100] flex items-center justify-center p-4 bg-black/60"
2238
initial={{ opacity: 0 }}
2339
animate={{ opacity: 1 }}
2440
exit={{ opacity: 0 }}
2541
transition={{ duration: 0.2 }}
2642
onClick={onClose}
43+
onKeyDown={e => e.key === 'Escape' && onClose()}
2744
role="dialog"
2845
aria-modal="true"
2946
aria-label={t('featureGate.title', { feature: featureLabel })}
47+
tabIndex={-1}
3048
>
3149
<motion.div
3250
className="bg-surface rounded-2xl shadow-2xl max-w-md w-full p-8"

src/components/SignInPromptModal.test.jsx

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ vi.mock('../hooks/useAuth', () => ({
1616

1717
const FEATURES = ['code', 'insight', 'export', 'sound', 'fullscreen'];
1818

19+
const FEATURE_LABELS = {
20+
code: 'Code Panel',
21+
insight: 'Algorithm Insight',
22+
export: 'Video Export',
23+
sound: 'Sound',
24+
fullscreen: 'Fullscreen Mode',
25+
};
26+
1927
describe('SignInPromptModal', () => {
2028
beforeEach(() => {
2129
signInWithGoogle.mockClear();
@@ -27,7 +35,11 @@ describe('SignInPromptModal', () => {
2735
<SignInPromptModal feature={featureKey} isOpen onClose={vi.fn()} />
2836
);
2937

30-
expect(screen.getByRole('dialog')).toBeInTheDocument();
38+
const dialog = screen.getByRole('dialog');
39+
expect(dialog).toBeInTheDocument();
40+
expect(dialog).toHaveAccessibleName(
41+
`Unlock ${FEATURE_LABELS[featureKey]}`
42+
);
3143
expect(screen.getByText('Sign in with Google')).toBeInTheDocument();
3244
expect(screen.getByText('Maybe later')).toBeInTheDocument();
3345
});
@@ -70,4 +82,24 @@ describe('SignInPromptModal', () => {
7082
fireEvent.click(overlay);
7183
expect(onClose).toHaveBeenCalledTimes(1);
7284
});
85+
86+
it('closes on Escape key', () => {
87+
const onClose = vi.fn();
88+
renderWithI18n(
89+
<SignInPromptModal feature="code" isOpen onClose={onClose} />
90+
);
91+
92+
fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' });
93+
expect(onClose).toHaveBeenCalledTimes(1);
94+
});
95+
96+
it('does not close on other keys', () => {
97+
const onClose = vi.fn();
98+
renderWithI18n(
99+
<SignInPromptModal feature="code" isOpen onClose={onClose} />
100+
);
101+
102+
fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Enter' });
103+
expect(onClose).not.toHaveBeenCalled();
104+
});
73105
});

0 commit comments

Comments
 (0)