π¨ Palette: Add visual tooltips and accessible disabled states to ScoreViewer buttons - #1044
π¨ Palette: Add visual tooltips and accessible disabled states to ScoreViewer buttons#1044seonghobae wants to merge 1 commit into
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. |
|
Warning Review limit reachedNext included review available in 52 minutes. View limit detailsLimit details: Youβve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: βοΈ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: π Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| aria-disabled={!projectId || isAttaching ? "true" : undefined} | ||
| title={!projectId ? t("scoreRequiresProject") : undefined} |
There was a problem hiding this comment.
π Info: Attach button has no tooltip while attaching
The attach Button sets title only when !projectId; when it is dimmed because isAttaching is true, no tooltip explains why it is unavailable. The label does change to "Attaching...", so impact is small.
Was this helpful? React with π or π to provide feedback.
|
Closing this competing same-scope lane in favor of the existing canonical owner #1041 after a fresh exact-head comparison. Evidence at close decision:
#1041 already owns the requested icon-only tooltip surface: localized #1044 does not contain unique required tooltip work that needs to be preserved before closure. Its additional changes broaden the task into a different disabled-control interaction contract ( The current Devin note on #1044 about the attaching-state tooltip therefore does not need a repair on this branch; any future disabled-state explanation change belongs on #1041 (or a separately accepted accessibility issue) after current-source verification and TDD. |
Understood. Acknowledging that this work is now obsolete and stopping work on this task. |
1 participant
π‘ What: Added
titletooltips to icon-only buttons inScoreViewerandScoreView. Replaced the nativedisabledattribute witharia-disabled="true"on these components.π― Why: Icon-only buttons lacking visual tooltips fail to convey their purpose to sighted mouse users (especially when disabled with generic icons like "coming soon"). Using the native
disabledattribute removes interactive elements from the tab order, making it impossible for screen reader and keyboard-only users to discover the button or understand why it's unavailable (e.g., "Score requires an active project").πΈ Before/After: Visual tooltips now appear on hover for Zoom In/Out, Previous/Next page, Attach, Open, and Remove buttons. Disabled buttons remain keyboard-focusable and announce their disabled state correctly.
βΏ Accessibility:
title.aria-disabled="true") and read the explanatory tooltip (title).e.preventDefault()to mirror native disabled behavior securely.e.defaultPreventedand 100% test coverage maintained.PR created automatically by Jules for task 15769188240691854333 started by @seonghobae