๐จ Palette: [UX improvement] Add accessible disabled states to submit buttons - #686
๐จ Palette: [UX improvement] Add accessible disabled states to submit buttons#686seonghobae wants to merge 1 commit into
Conversation
โฆy updates you requested. Here is a breakdown of the changes I made: ๐จ Palette: [UX improvement] Add accessible disabled states to form buttons ๐ก What: - Removed native `disabled` attributes from the primary form buttons in `AddTableModal` and `GroupModal`. - Added `aria-disabled` and `title` attributes for tooltips on disabled buttons. - Added explicit `onClick` handlers to prevent form processing when the button is visually disabled. - Updated visual styling to replicate the disabled state (`opacity: 0.5, cursor: "not-allowed"`). - Updated corresponding test assertions in `ModalCoverage.test.tsx` to check `aria-disabled`. ๐ฏ Why: Native `disabled` buttons are removed from the browser's tab order, making them invisible to screen readers and preventing users from discovering why the button is disabled via tooltips. Using `aria-disabled` solves this, but requires manual intervention to block the actions. ๐ธ Before/After: Before: `disabled` attribute used. Tooltips wouldn't show. After: `aria-disabled` used with custom styles and `onClick` handlers. Tooltips appear on hover/focus. โฟ Accessibility: Screen readers now announce the buttons as disabled, and keyboard-only users can tab to the buttons to read the tooltips explaining the required actions. Let me know if you need any further adjustments!
|
๐ Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a ๐ emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
๐ WalkthroughWalkthrough๋ ๋ชจ๋ฌ์ ๋นํ์ฑ ๋ฒํผ์ด Changes๋ชจ๋ฌ ๋ฒํผ ์ ๊ทผ์ฑ
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
๐ฅ Pre-merge checks | โ 5โ Passed checks (5 passed)
โจ Finishing Touches๐ Generate docstrings
๐งช Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
๐งน Nitpick comments (1)
frontend/src/components/modals/ModalCoverage.test.tsx (1)
86-86: ๐ฏ Functional Correctness | ๐ต Trivial | โก Quick win๋นํ์ฑ ์ํ์ ์ค์ ์ ์ถ ์ฐจ๋จ๋ ํ ์คํธํ์ธ์.
ํ์ฌ ํ ์คํธ๋
aria-disabled๊ฐ๋ง ํ์ธํ๋ฏ๋ก, ํด๋ฆญ ๋๋ ํผ ์ ์ถ ์ฐจ๋จ ๋ก์ง์ด ์ ๊ฑฐ๋์ด๋ ํต๊ณผํฉ๋๋ค. ๋น/๊ณต๋ฐฑ ์ด๋ฆ์์ ํด๋ฆญ๊ณผ Enter ์ ์ถ ์ callback์ด ํธ์ถ๋์ง ์๋์ง, ์ ํจํ ์ด๋ฆ์์๋ callback์ด ํธ์ถ๋๋์ง๋ ๊ฒ์ฆํด ์ฃผ์ธ์.Also applies to: 245-245
๐ค Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/modals/ModalCoverage.test.tsx` at line 86, Update the ModalCoverage tests around the save-button assertions to verify actual submission behavior, not only the aria-disabled attribute. Add cases confirming blank and whitespace-only names do not invoke the callback when clicking the button or submitting with Enter, and that a valid name does invoke the callback.
๐ค Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/src/components/modals/ModalCoverage.test.tsx`:
- Line 86: Update the ModalCoverage tests around the save-button assertions to
verify actual submission behavior, not only the aria-disabled attribute. Add
cases confirming blank and whitespace-only names do not invoke the callback when
clicking the button or submitting with Enter, and that a valid name does invoke
the callback.
โน๏ธ Review info
โ๏ธ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 46a33222-f44f-4d3f-b64b-0fed21d0d180
๐ Files selected for processing (4)
.jules/palette.mdfrontend/src/components/modals/AddTableModal.tsxfrontend/src/components/modals/GroupModal.tsxfrontend/src/components/modals/ModalCoverage.test.tsx
๐จ Palette: [UX improvement] Add accessible disabled states to submit buttons
๐ก What:
disabledattributes from submit buttons inAddTableModalandGroupModal.aria-disabledandtitleattributes for tooltips on disabled buttons.onClickhandlers to prevent form submission when the button is visually disabled.opacity: 0.5, cursor: "not-allowed").ModalCoverage.test.tsxto checkaria-disabled.๐ฏ Why:
Native
disabledbuttons are removed from the browser's tab order, making them invisible to screen readers and preventing users from discovering why the button is disabled via tooltips. Usingaria-disabledsolves this, but requires manual intervention to block submissions.๐ธ Before/After:
Before:
disabledattribute used. Tooltips wouldn't show.After:
aria-disabledused with custom styles andonClickhandlers. Tooltips appear on hover/focus.โฟ Accessibility:
Screen readers now announce the buttons as disabled, and keyboard-only users can tab to the buttons to read the tooltips explaining the required actions.
PR created automatically by Jules for task 1031071165122331047 started by @seonghobae
Summary by CodeRabbit
์ ๊ทผ์ฑ ๊ฐ์
aria-disabled์ ์๋ด ํดํ์ ์ถ๊ฐํ์ต๋๋ค.ํ ์คํธ