fix(ui-instructure): use CondensedButton for AI component triggers#2636
Conversation
|
@adamlobler I add you as a reviewer too, please check if CondensedButton is a good choice from a design perspective. |
There was a problem hiding this comment.
Pull request overview
Updates AI component modal triggers in ui-instructure to use semantic buttons (via CondensedButton) instead of anchor links, aligning trigger semantics with accessibility expectations for actions that open dialogs.
Changes:
- Replaced
Link-based modal triggers (withhref="#"andpreventDefault) withCondensedButtontriggers. - Updated
@instructure/ui-buttonsimports to includeCondensedButtonand removed@instructure/ui-linkimports in both v1 (v11_6) and v2 (latest) implementations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/ui-instructure/src/NutritionFacts/v2/index.tsx | Swaps modal trigger from Link to CondensedButton for v2. |
| packages/ui-instructure/src/NutritionFacts/v1/index.tsx | Swaps modal trigger from Link to CondensedButton for v1 (v11_6). |
| packages/ui-instructure/src/DataPermissionLevels/v2/index.tsx | Swaps modal trigger from Link to CondensedButton for v2. |
| packages/ui-instructure/src/DataPermissionLevels/v1/index.tsx | Swaps modal trigger from Link to CondensedButton for v1 (v11_6). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| forceButtonRole={false} | ||
| href="#" | ||
| > | ||
| <CondensedButton onClick={() => setOpen(true)}> |
| forceButtonRole={false} | ||
| href="#" | ||
| > | ||
| <CondensedButton onClick={() => setOpen(true)}> |
| forceButtonRole={false} | ||
| href="#" | ||
| > | ||
| <CondensedButton onClick={() => setOpen(true)}> |
| forceButtonRole={false} | ||
| href="#" | ||
| > | ||
| <CondensedButton onClick={() => setOpen(true)}> |
|
Visual regression report✅ No changes.
Baselines come from the |
HerrTopi
left a comment
HerrTopi
left a comment
There was a problem hiding this comment.
consider the ai's suggestion about the aria dialog. Otherwise, looks good
Link is changed to CondensedButton to comply with WCAG rules.
Test: