๐จ Palette: [UX improvement] ์ค์ฝ์ด ๋ทฐ์ด ๋ด ์์ด์ฝ ๋ฒํผ ๋ฐ ๋นํ์ฑํ ์ํ ํดํ ๊ฐ์ - #699
Conversation
|
๐ 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
์ด PR์ ๋ฐ์คํฌํฑ ์ฑ์ ์ ๋ณด(Score) UI์์ ์์ด์ฝ ๋ฒํผ ํดํ(title) ์ ๊ณต๊ณผ ๋นํ์ฑ ์ํ ์ ๊ทผ์ฑ(aria-disabled) ๊ฐ์ ์ ํตํด ๋ง์ฐ์ค/์คํฌ๋ฆฐ๋ฆฌ๋ ์ฌ์ฉ์ฑ์ ๋์ด๊ธฐ ์ํ ๋ณ๊ฒฝ์ ๋๋ค.
Changes:
ScoreViewer์ ์์ด์ฝ ๋ฒํผ(ํ๋/์ถ์, ์ด์ /๋ค์ ํ์ด์ง)์title์ ์ถ๊ฐํด ๋ธ๋ผ์ฐ์ ๊ธฐ๋ณธ ํดํ์ด ํ์๋๋๋ก ๊ฐ์ScoreView์ โ์ ๋ณด ์ด๊ธฐโ ๋ฒํผ์disabled๋์aria-disabled๋ก ์ ํํ๊ณ ๋นํ์ฑ ์ฌ์title์ ์ถ๊ฐScoreView๋จ์ ํ ์คํธ์์disabled๊ธฐ๋ฐ ๊ฒ์ฆ์aria-disabled๊ธฐ๋ฐ์ผ๋ก ์์
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/desktop/src/features/score/ScoreViewer.tsx | ์์ด์ฝ ๋ฒํผ์ title์ ์ถ๊ฐํด ๋ง์ฐ์ค ํธ๋ฒ ์ ๊ธฐ๋ฅ ํดํ ์ ๊ณต |
| apps/desktop/src/features/score/ScoreView.tsx | โ์ด๊ธฐโ ๋ฒํผ์ aria-disabled๋ก ์ ํ + ๋นํ์ฑ ์ฌ์ ํดํ ์ถ๊ฐ(๋จ, โ์ญ์ โ ๋ฒํผ์ ์ฌ์ ํ disabled ์ฌ์ฉ) |
| apps/desktop/src/features/score/ScoreView.test.tsx | โ์ด๊ธฐโ ๋ฒํผ ๊ฒ์ฆ์ aria-disabled="true" ๊ธฐ์ค์ผ๋ก ๋ณ๊ฒฝ |
Comments suppressed due to low confidence (1)
apps/desktop/src/features/score/ScoreView.tsx:202
- ์ญ์ ์์ด์ฝ ๋ฒํผ์
title์ ์ถ๊ฐํ์ง๋ง,disabled={!projectId}๋๋ฌธ์ ๋ธ๋ผ์ฐ์ ๊ธฐ๋ณธ ํดํ์ด ํ์๋์ง ์์ต๋๋ค(๋ค์ดํฐ๋ธ disabled๋ ํฌ์ธํฐ ์ด๋ฒคํธ๋ฅผ ์ฐจ๋จ). ์์ ์ด๊ธฐ ๋ฒํผ๊ณผ ๋์ผํ๊ฒaria-disabled๋ก ํ์ํ๊ณ ,projectId๊ฐ ์์ ๋๋ ํด๋ฆญ์ด ์๋ฌด ๋์๋ ํ์ง ์๋๋ก ์ฒ๋ฆฌํด์ผ ๋นํ์ฑํ ์ฌ์ ํดํ์ด ์ค์ ๋ก ๋์ํฉ๋๋ค.
onClick={projectId ? () => void handleRemove(projectId, attachment) : undefined}
disabled={!projectId}
aria-label={`${t("scoreRemove")}: ${attachment.fileName}`}
title={!projectId ? t("scoreRequiresProject") : `${t("scoreRemove")}: ${attachment.fileName}`}
๐ก Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type="button" | ||
| onClick={projectId ? () => void openAttachment(projectId, attachment) : undefined} | ||
| disabled={!projectId} | ||
| onClick={projectId ? () => void openAttachment(projectId, attachment) : (e) => e.preventDefault()} |
- package-lock.json์์ brace-expansion, postcss ๋ฑ์ ์ทจ์ฝ์ ํจ์น (npm audit fix) - services/analysis-engine/uv.lock์์ yt-dlp, setuptools ๋ณด์ ์ ๋ฐ์ดํธ - ScoreView, ScoreViewer ๋ฒํผ์ aria-disabled์ tooltip(title) ์ถ๊ฐํ์ฌ ์ ๊ทผ์ฑ ํฅ์ - ๊ด๋ จ ํ ์คํธ ์ฝ๋(aria-disabled ์์ฑ ๊ฒ์ฆ) ์์
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
apps/desktop/src/features/score/ScoreView.tsx:186
type="button"elements donโt have a meaningful default action to cancel, soe.preventDefault()is a no-op here. Keeping a dedicated handler for the disabled state makes the intent harder to read; you can omit the handler entirely and rely onaria-disabled+ the absence ofonClickto prevent activation.
onClick={projectId ? () => void openAttachment(projectId, attachment) : (e) => e.preventDefault()}
| onClick={projectId ? () => void handleRemove(projectId, attachment) : undefined} | ||
| disabled={!projectId} | ||
| aria-label={`${t("scoreRemove")}: ${attachment.fileName}`} | ||
| title={!projectId ? t("scoreRequiresProject") : `${t("scoreRemove")}: ${attachment.fileName}`} |
- `services/analysis-engine/uv.lock`์ setuptools ํจํค์ง ์ ๊ทธ๋ ์ด๋๋ฅผ ํตํด CVE-2026-59890 ๋์ - lockfile ๋๊ธฐํ๋ฅผ ํตํด trivy-fs ์ค์บ ํต๊ณผ ํ๋ณด
- services/analysis-engine/pyproject.toml์ setuptools ๋ฒ์ ์ >=75.0.0์ผ๋ก ๊ณ ์ ํ์ฌ ์ทจ์ฝ์ ๋ฐฉ์ง - uv lock ๊ฐฑ์ ์ผ๋ก CI ์ค์บ ํต๊ณผ ํ๋ณด
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
apps/desktop/src/features/score/ScoreView.tsx:188
- When
projectIdis missing, theonClickhandler callse.preventDefault(), buttype="button"means thereโs no default action to cancel. This adds an unnecessary handler and still leaves the control activatable (even though it does nothing). Prefer omittingonClickentirely when the action is disabled.
type="button"
onClick={projectId ? () => void openAttachment(projectId, attachment) : (e) => e.preventDefault()}
aria-disabled={!projectId ? true : undefined}
title={!projectId ? t("scoreRequiresProject") : undefined}
apps/desktop/src/features/score/ScoreView.tsx:203
titletooltips wonโt appear for a disabled<button>in most browsers because disabled controls donโt receive pointer events. Currently the remove icon button setstitleto the disabled-reason string whendisabled={!projectId}, but that tooltip wonโt be shown when the button is actually disabled. If you want a hover tooltip in the disabled state, put the disabled-statetitleon a non-disabled wrapper element (or switch this control to the samearia-disabledpattern used above).
onClick={projectId ? () => void handleRemove(projectId, attachment) : undefined}
disabled={!projectId}
aria-label={`${t("scoreRemove")}: ${attachment.fileName}`}
title={!projectId ? t("scoreRequiresProject") : `${t("scoreRemove")}: ${attachment.fileName}`}
className="size-10 border-rose-300/25 text-rose-200 hover:bg-rose-400/10"
| "numpy>=1.26", | ||
| "setuptools>=75.0.0", | ||
| "soundfile>=0.13.1", | ||
| "urllib3>=2.7.0", | ||
| "urllib3>=2.7.0", | ||
| "yt-dlp>=2026.6.9", |
|
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. |
Acknowledged. |
๐ก What:
ScoreView.tsx์ ์ ๋ณด ์ฒจ๋ถ/์ด๊ธฐ ๋ฒํผ์์disabled์์ฑ ๋์aria-disabled๋ฅผ ์ฌ์ฉํ์ฌ ์คํฌ๋ฆฐ ๋ฆฌ๋ ์ ๊ทผ์ฑ์ ๊ฐ์ ํ๊ณ , ๋นํ์ฑํ ์ฌ์ ๋ฅผ ์ค๋ช ํ๋titleํดํ์ ์ถ๊ฐํ์ต๋๋ค.ScoreView.tsx์ScoreViewer.tsx์ ์๋ ์์ด์ฝ ์ ์ฉ ๋ฒํผ(์ญ์ , ํ๋/์ถ์, ์ด์ /๋ค์ ํ์ด์ง)์title์์ฑ์ ์ถ๊ฐํ์ฌ ๋ง์ฐ์ค ์ฌ์ฉ์์๊ฒ ๋ธ๋ผ์ฐ์ ๊ธฐ๋ณธ ํดํ์ด ์ ๊ณต๋๋๋ก ํ์ต๋๋ค.ScoreView.test.tsx)์์toBeDisabled๋ก ์์ฑ๋์๋ ์ฝ๋๋ฅผtoHaveAttribute("aria-disabled", "true")๋ก ์์ ํ์ฌ ํ ์คํธ๊ฐ ํต๊ณผํ๋๋ก ๋ฐ์ํ์ต๋๋ค.๐ฏ Why:
disabled์์ฑ์ ์์๋ฅผ ์คํฌ๋ฆฐ ๋ฆฌ๋์์ ์์ ํ ์จ๊ธฐ๊ณ ํฌ์ธํฐ ์ด๋ฒคํธ๋ฅผ ์ฐจ๋จํ์ฌ ํดํ์ด ๋์ํ์ง ์๊ฒ ํ๋ ๋ฌธ์ ๊ฐ ์์์ต๋๋ค. ์ด๋ฅผaria-disabled๋ก ๋ณ๊ฒฝํ์ฌ ํค๋ณด๋ ํฌ์ปค์ค์ ํดํ ์ ๊ทผ์ฑ์ ๋ณด์ฅํฉ๋๋ค.title์์ฑ์ด ๋๋ฝ๋์ด ์์ด ์๊ฐ ์ฅ์ ๊ฐ ์๋ ๋ง์ฐ์ค ์ฌ์ฉ์๊ฐ ๋ฒํผ์ ๊ธฐ๋ฅ์ ๋ช ํํ ์ธ์งํ๊ธฐ ์ด๋ ค์ ์ต๋๋ค.๐ธ Before/After:
โฟ Accessibility:
title) ์ง์์ ์ถ๊ฐํ์ฌ ๋ง์ฐ์ค ์ฌ์ฉ์ ๊ฒฝํ ํฅ์.PR created automatically by Jules for task 4854348697760410680 started by @seonghobae