π¨ Palette: μ 보 λ·°μ΄ μ κ·Όμ± λ° ν΄ν κ°μ - #697
Conversation
μμ΄μ½ μ μ© λ²νΌ(νλ/μΆμ, νμ΄μ§ μ΄λ, μμ λ±)μ `title` μμ±μ μΆκ°νμ¬ λ€μ΄ν°λΈ ν΄νμ μ 곡νλλ‘ κ°μ νμ΅λλ€. λν `disabled` μμ±μ `aria-disabled`λ‘ λ³κ²½νκ³ ν΄λ¦ μ΄λ²€νΈλ₯Ό μ°¨λ¨νμ¬ μ€ν¬λ¦° 리λ μ¬μ©μμ λ§μ°μ€ μ¬μ©μ λͺ¨λμκ² μΌκ΄λ μ κ·Όμ±μ μ 곡ν©λλ€.
|
π 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. |
There was a problem hiding this comment.
Pull request overview
This PR improves the score viewerβs icon-button accessibility and usability by adding localized tooltips (title) and replacing native disabled with aria-disabled + click prevention so tooltips and assistive tech behaviors remain available.
Changes:
- Added
titleattributes to icon-only controls inScoreViewer(zoom, fit-to-width, prev/next page) and the βremove scoreβ action inScoreView. - Replaced native
disabledusage witharia-disabledand guardedonClickhandlers to prevent actions when βdisabledβ. - Updated React Testing Library assertions to validate
aria-disabledand verify click default-prevention at bounds.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/desktop/src/features/score/ScoreViewer.tsx | Adds tooltips and converts page navigation buttons from disabled to aria-disabled with guarded click handling. |
| apps/desktop/src/features/score/ScoreViewer.test.tsx | Updates tests to assert aria-disabled and verify default-prevented clicks at page bounds. |
| apps/desktop/src/features/score/ScoreView.tsx | Converts βAdd/Open/Removeβ score actions to aria-disabled with guarded click handling; adds tooltip for remove action. |
| apps/desktop/src/features/score/ScoreView.test.tsx | Updates tests to assert aria-disabled instead of native disabled state. |
Comments suppressed due to low confidence (1)
apps/desktop/src/features/score/ScoreView.tsx:220
- Because this button no longer uses the native
disabledattribute, thehover:bg-rose-400/10class will still trigger whenaria-disabled="true", which can make a disabled destructive action look clickable. Add anaria-disabled:hover:*override to neutralize hover styling in the disabled state.
className="size-10 border-rose-300/25 text-rose-200 hover:bg-rose-400/10"
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| aria-disabled={!projectId || isAttaching} | ||
| variant="secondary" | ||
| className="min-h-11 border border-cyan-300/20 bg-cyan-300/10 font-semibold text-cyan-50 hover:bg-cyan-300/20" | ||
| className="min-h-11 border border-cyan-300/20 bg-cyan-300/10 font-semibold text-cyan-50 hover:bg-cyan-300/20 aria-disabled:opacity-50 aria-disabled:cursor-not-allowed" |
μμ΄μ½ μ μ© λ²νΌ(νλ/μΆμ, νμ΄μ§ μ΄λ, μμ λ±)μ `title` μμ±μ μΆκ°νμ¬ λ€μ΄ν°λΈ ν΄νμ μ 곡νλλ‘ κ°μ νμ΅λλ€. λν `disabled` μμ±μ `aria-disabled`λ‘ λ³κ²½νκ³ ν΄λ¦ μ΄λ²€νΈλ₯Ό μ°¨λ¨νμ¬ μ€ν¬λ¦° 리λ μ¬μ©μμ λ§μ°μ€ μ¬μ©μ λͺ¨λμκ² μΌκ΄λ μ κ·Όμ±μ μ 곡ν©λλ€. μμ‘΄μ± μ·¨μ½μ (npm audit λ° trivy-fs μλ¬) μμ μ ν¬ν¨ν©λλ€.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
apps/desktop/src/features/score/ScoreView.tsx:146
aria-disabledis used to simulate a disabled state, but this button still applies the active hover background (hover:bg-cyan-300/20) even whenaria-disabled="true". That gives a misleading βinteractiveβ affordance in the disabled state and undermines the accessibility intent of the change. Consider overriding hover styles when aria-disabled is true.
aria-disabled={!projectId || isAttaching}
variant="secondary"
className="min-h-11 border border-cyan-300/20 bg-cyan-300/10 font-semibold text-cyan-50 hover:bg-cyan-300/20 aria-disabled:opacity-50 aria-disabled:cursor-not-allowed"
| aria-disabled={!projectId} | ||
| aria-label={`${t("scoreRemove")}: ${attachment.fileName}`} | ||
| title={`${t("scoreRemove")}: ${attachment.fileName}`} | ||
| className="size-10 border-rose-300/25 text-rose-200 hover:bg-rose-400/10" |
| aria-current={selected?.id === attachment.id ? "true" : undefined} | ||
| aria-label={`${t("scoreOpen")}: ${attachment.fileName}`} | ||
| className="flex min-h-10 min-w-0 flex-1 items-center gap-2 text-left text-sm font-semibold text-slate-100 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300 disabled:cursor-not-allowed disabled:opacity-60" | ||
| className="flex min-h-10 min-w-0 flex-1 items-center gap-2 text-left text-sm font-semibold text-slate-100 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300 disabled:cursor-not-allowed disabled:opacity-60 aria-disabled:cursor-not-allowed aria-disabled:opacity-60" |
μμ΄μ½ μ μ© λ²νΌ(νλ/μΆμ, νμ΄μ§ μ΄λ, μμ λ±)μ `title` μμ±μ μΆκ°νμ¬ λ€μ΄ν°λΈ ν΄νμ μ 곡νλλ‘ κ°μ νμ΅λλ€. λν `disabled` μμ±μ `aria-disabled`λ‘ λ³κ²½νκ³ ν΄λ¦ μ΄λ²€νΈλ₯Ό μ°¨λ¨νμ¬ μ€ν¬λ¦° 리λ μ¬μ©μμ λ§μ°μ€ μ¬μ©μ λͺ¨λμκ² μΌκ΄λ μ κ·Όμ±μ μ 곡ν©λλ€. trivy-fs CI μλ¬λ₯Ό ν΄κ²°νκΈ° μν΄ setuptoolsμ yt-dlpμ λ²μ μ uv.lockμ μ κ·Έλ μ΄λ νμμ΅λλ€.
μμ΄μ½ μ μ© λ²νΌ(νλ/μΆμ, νμ΄μ§ μ΄λ, μμ λ±)μ `title` μμ±μ μΆκ°νμ¬ λ€μ΄ν°λΈ ν΄νμ μ 곡νλλ‘ κ°μ νμ΅λλ€. λν `disabled` μμ±μ `aria-disabled`λ‘ λ³κ²½νκ³ ν΄λ¦ μ΄λ²€νΈλ₯Ό μ°¨λ¨νμ¬ μ€ν¬λ¦° 리λ μ¬μ©μμ λ§μ°μ€ μ¬μ©μ λͺ¨λμκ² μΌκ΄λ μ κ·Όμ±μ μ 곡ν©λλ€. trivy-fs CI μλ¬λ₯Ό ν΄κ²°νκΈ° μν΄ setuptools λ²μ μ uv.lockμ 83.0.0 μΌλ‘ μ κ·Έλ μ΄λ νμμ΅λλ€.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 7 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
apps/desktop/src/features/score/ScoreViewer.tsx:323
- Same as the previous button: prefer omitting aria-disabled when enabled (undefined) to match existing patterns (and avoid emitting
aria-disabled="false").
className="size-14"
aria-label={t("scoreViewerNextPage")}
title={t("scoreViewerNextPage")}
aria-disabled={pageNumber >= pageCount}
onClick={(e) => {
if (pageNumber >= pageCount) {
e.preventDefault();
return;
apps/desktop/src/features/score/ScoreView.tsx:203
- This button no longer uses the native
disabledattribute, so thedisabled:*Tailwind variants in this className will never apply (and can be misleading). Consider removing thedisabled:*variants now that aria-disabled is the mechanism in use.
aria-disabled={!projectId}
aria-current={selected?.id === attachment.id ? "true" : undefined}
aria-label={`${t("scoreOpen")}: ${attachment.fileName}`}
className="flex min-h-10 min-w-0 flex-1 items-center gap-2 text-left text-sm font-semibold text-slate-100 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300 disabled:cursor-not-allowed disabled:opacity-60 aria-disabled:cursor-not-allowed aria-disabled:opacity-60"
>
| aria-disabled={pageNumber <= 1} | ||
| onClick={(e) => { | ||
| if (pageNumber <= 1) { | ||
| e.preventDefault(); | ||
| return; | ||
| } |
| } | ||
| void handleAttach(projectId); | ||
| }} | ||
| aria-disabled={!projectId || isAttaching} |
|
Closing as closed-obsolete (palette a11y micro-PR storm) by autonomous product-completion loop. Winner / disposition target: #727 Unique value from this cluster is consolidated there (projectId path guard + npm HIGH CVE pins + Foote novelty kernel sign). Remaining micro-duplicates do not land additional commercial readiness. |
|
closed-closed-obsolete (palette a11y micro-PR storm) β #727 |
Understood. Acknowledging that this work is now obsolete and stopping work on this task. |
Understood. Acknowledging that this work is now obsolete and stopping work on this task. |
π‘ What
μ 보 λ·°μ΄ νλ©΄μ μμ΄μ½ λ²νΌ(νλ/μΆμ, νμ΄μ§ μ΄λ, μ 보 μμ λ±)μ
titleμμ±μ μΆκ°νκ³ λ€μ΄ν°λΈdisabledμμ±μaria-disabled="true"λ‘ λ³κ²½νμ΅λλ€.π― Why
titleν΄νμ μ 곡νμ¬ μ§κ΄μ±μ λμμ΅λλ€.disabledμμ±μ μ€ν¬λ¦° 리λμμ μμλ₯Ό μμ ν μ¨κΈ°κ³ λ§μ°μ€ μ΄λ²€νΈ(νΈλ² ν΄ν λ±)λ₯Ό μ°¨λ¨νλ―λ‘,aria-disabledμe.preventDefault()λ₯Ό μ¬μ©νμ¬ ν€λ³΄λ λ° μ€ν¬λ¦° 리λ μ κ·Όμ±μ μ μ§νλ©΄μ ν΄νμ΄ μλνλλ‘ κ°μ νμ΅λλ€.βΏ Accessibility
titleλ°aria-labelμΌμΉdisabledμμ± μ κ±° λ°aria-disabledλμ μΌλ‘ μ κ·Όμ± ν₯μaria-disabledκ²μ¦ λ°©μμΌλ‘ μ λ°μ΄νΈνκ³ μ΄λ²€νΈ μ°¨λ¨ ν μ€νΈ μΆκ°PR created automatically by Jules for task 7377617624541599206 started by @seonghobae