[Remove Vuetify from Studio] Content Library Catalog - Frequently asked questions page #5517
Conversation
I found a broken link while testing. |
|
Thanks for your continued work @yeshwanth235, very high-level, it's aligned with expectations. We will assign someone for a detailed review next week. |
|
@yeshwanth235 Could you merge latest |
|
And thanks for reporting the link, I will find out where it was supposed to go. |
|
@yeshwanth235 as for the broken link, the right target for now will be |
Updated the link @MisRob. I have taken care |
MisRob
changed the title
akolson
left a comment
akolson
left a comment
There was a problem hiding this comment.
Hi @yeshwanth235! Great work on this PR. Could you please take some time to implement the accessibility requirements outlined in the issue? It looks like they haven’t been addressed yet. Once that’s done, I’ll be able to do a more comprehensive review of the changes as a whole. Thanks!
Sure, will check and update |
| type="button" | ||
| :aria-expanded="showAccordionItem.toString()" | ||
| :aria-controls="id" | ||
| class="studio-accordion-header" |
There was a problem hiding this comment.
Looks like the focus ring for the accordions is yellow 🙂
We could easily resolve this by adding our KDS outline using a computed property and using it here additionally;
<button
...
:class="someComputedPropName"
class="studio-accordion-header"
....
....
computed: {
someComputedPropName() {
return this.$computedClass({
':focus': { ...this.$coreOutline, outlineOffset: 0 },
});
},
}
There was a problem hiding this comment.
Hi @yeshwanth235! Great work on migrating the accordions to KDS in this pr! Here's a few issues I picked up so we can have a more solid implementation that is accessible
- Although the page is responsive, it appears the text alignment has regressed.
| Before (Studio/hotfixes) | After (Implementation) |
|---|---|
 |
 |
-
I noticed that a regular
<button>was used to implement the accordion. I’m wondering if usingKButtonwithappearance="basic-link"—and customizing it throughappearanceOverrides—might be feasible. This could remove the need to use$coreOutline, sinceKButtonis already accessible. This is just a thought and not a blocker for the progress of this PR unless it turns out to be practical. -
I've also added a few a11y comments to consider.
| :id="id" | ||
| type="button" | ||
| :aria-expanded="showAccordionItem.toString()" | ||
| :aria-controls="id" |
There was a problem hiding this comment.
- We have a binding of the
idon the button but not on the panel being controlled. aria-controls must reference the id of the element that is being shown/hidden (the panel). Can't seem to see any suggestion of this incontentcuration/contentcuration/frontend/shared/views/StudioAccordionItem.vue
There was a problem hiding this comment.
It might be worth adding the aria-labelledby to the panel so screen readers can know the panel's label. Optionally we could use the role="region" for better semantics.
| }, | ||
| id: { | ||
| type: String, | ||
| default: 'studio-accordion', |
There was a problem hiding this comment.
Because this isn't set and is defaulted to studio accordion, the aria relationships break due to ID duplication
3 participants
Summary
Removed all Vuetify components and migrated to KDS components in /faq
Created StudioAccordion and StudioAccordionItem for accordion.
Video: link
References
Fixes 5502
Reviewer guidance
Need full page testing for /faq in web and mweb