-
Notifications
You must be signed in to change notification settings - Fork 401
Add “expandable card/region” example to disclosure pattern #3251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Heya @mcking65, here’s an initial PR for the expandable region pattern. There are a number of linting errors I’ll need to resolve, but I may not have time to do that before I head out early next week. @howard-e, could you let me know if the preview build failure is being caused by those linting errors? If so, I’ll do my best to get those fixed before I leave. In the meantime, please feel free to review and share feedback for this standalone Codepen. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@howard-e, could you let me know if the preview build failure is being caused by those linting errors? If so, I’ll do my best to get those fixed before I leave.
@adampage nope, the linting isn't the culprit here.
The preview generation builder expects files and folders to be in a very specific structure, seemingly to avoid any conflicts or invalid data-to-template attachments. One of those is having a defined *-pattern.html
in the immediate sub-directory of the /patterns
directory. So the expectation would be to have a /content/patterns/expandable-region/expandable-region-pattern.html
(along with the references to it).
This is obviously a pain point to newer contributors (apologies) so there is an existing issue I'm hoping to get to shortly to document these particularities (or to revise altogether).
But that restriction can be ignored, which I have in w3c/wai-aria-practices#395 to allow this to successfully build. In this approach however, it won’t include the "WAI" wrapper around it, just the html, so I’ve already prepared a PR to remove it when this rename is done.
content/patterns/expandable-region/examples/expandable-region.html
Outdated
Show resolved
Hide resolved
…html Co-authored-by: Howard Edwards <howarde.edwards@gmail.com>
Co-authored-by: Howard Edwards <howarde.edwards@gmail.com>
I’m suspicious of these HTML linting errors. 🤔
|
The ARIA Authoring Practices (APG) Task Force just discussed The full IRC log of that discussion<jugglinmike> Topic: PR 3251 - New expandable region pattern<jugglinmike> github: https://github.com//pull/3251 <jugglinmike> Matt_King: Did we want this to be a separate pattern, or would it be another form of disclosure? <jugglinmike> Matt_King: It could be one example that's under the "disclosure" pattern, but if it is a truly distinct pattern, then I'm happy to have a separate page for it <jugglinmike> Matt_King: ...in that case, as howard-e was explaining, we need to have a "pattern" file and a sub-directory for the example, and an example file <jugglinmike> Matt_King: Though even then, we will probably make a note that it is related to the "disclosure" pattern <jugglinmike> Adam_Page: It hadn't occurred to me to add it as an example of an existing pattern <jugglinmike> Adam_Page: I don't have a strong opinion on this one. I added it as a pattern only because it was originally pitched that way. I don't think we ever really discussed if that was the best option <jugglinmike> Adam_Page: I've updated this with howard-e's help so that it builds as it should <jugglinmike> Adam_Page: Basically, I did the initial implementation and that was it. I am mainly looking for feedback on the example itself, knowing that I will need to go back and fill out the documentation (and image, etc.) accordingly <Jem> https://deploy-preview-396--aria-practices.netlify.app/aria/apg/patterns/expandable-region/examples/expandable-region/ <jugglinmike> Adam_Page: There is some appeal to me in making this another example within the "disclosure" pattern. It really is a disclosure <jugglinmike> Adam_Page: Again, I don't have strong opinions on this, and I wasn't present for the original discussion <jugglinmike> Adam_Page: But if I understand the impetus, this was intended as a sort of preferable way for the use case that many people were satisfying by stuffing a lot of stuff into buttons <Jem> Jaws has imposter syndrom. LOL <jugglinmike> Matt_King: When I tried to collapse, JAWS lost its place. It went to "impostor syndrome". The performance isn't great, either <jugglinmike> Matt_King: I don't know what JAWS is doing here, actually... <jugglinmike> Adam_Page: I haven't tested in JAWS. I developed in Mac and used VoiceOver. It all seems to be working, there <jugglinmike> Matt_King: Calling this an "expandable region" is kind of interesting to me <jugglinmike> Matt_King: There's a region, and it has a little bit of content inside of it, and it has this option to make the content inside the region bigger <jugglinmike> Siri: It looks like an accordion, no? <jugglinmike> Adam_Page: It really is, fundamentally, a disclosure--like, an accordion <jugglinmike> Adam_Page: The quirk is that the thing that visually is inviting the user to expand is structurally rich. We're using JavaScript to enlarge the clickable region <jugglinmike> Jem: I don't think this is an accordion design pattern. This is a disclosure. <jugglinmike> Matt_King: I don't understand why it has anything to do with the region <jugglinmike> Matt_King: The "details" button feels like an ordinary disclosure <jugglinmike> Matt_King: So there's something about the visual presentation that says you can click on a larger area <jugglinmike> Matt_King: If I wanted to code this, I would say, "here is a landmark region that says 'fire to flare'" and then within that region, I would place a disclosure <jugglinmike> Siri: If we have to provide a region for the container inside, I don't think we need to go into that much detail <jugglinmike> Matt_King: It feels to me like this goes in the "disclosure" pattern <jugglinmike> Adam_Page: It certainly has a lot in common with "disclosure". If we were to make it its own page, then that new page would have a lot in common with "disclosure". It would maybe be redundant <jugglinmike> Adam_Page: Maybe we just call it a "rich disclosure". The differentiating factor is that the thing which expand the disclosure is rich--it is visually showing much more <jugglinmike> Matt_King: Right, but from a screen reader user's point of view, there is no difference <jugglinmike> Adam_Page: Yeah <jugglinmike> Adam_Page: That's where I was interested to get some feedback on the example. That's where I made some editorial decisions about what the screen reader should be <jugglinmike> Jem: What screen reader user experience are you referring to? <jugglinmike> Matt_King: It's different from the accordion because there, we use a heading and make the whole heading clickable. <jugglinmike> Matt_King: You can do the same thing with the accordion pattern. You could show more content than the heading in its collapsed state. <jugglinmike> Matt_King: That's feasible, and it's covered in the pattern <jugglinmike> Matt_King: In this case, this is a region with some content in it. A region that contains a heading and some text. And then following the text, there is a disclosure button. As a screen reader user, it doesn't feel any different from any other instance of a disclosure <jugglinmike> Matt_King: The reason they wanted it in the APG has to do with the approach to coding it, where you have a "section" element, and the idea is that everything inside of that section is clickable. <jugglinmike> Adam_Page: I think it was especially a reaction to how, in the real world, people are stuffing tons of stuff full of stuff that they shouldn't (and risking access issues) just to make that entire surface clickable <jugglinmike> Matt_King: So the alternative is that some people might use a button where you have a section? <jugglinmike> Adam_Page: Yes, that some people might stuff everything in my regions into a button <Daniel> q+ <jugglinmike> Matt_King: So the idea is to replace the button with a clickable section, and say "here's how you do it" <jugglinmike> Adam_Page: And to do that without inventing anything new for ARIA or HTML--just to solve the use case with things that already exist <jugglinmike> Matt_King: It looks to me like what you have for the details is a button element that doesn't look at all different from any of our "disclosure" buttons that we already have in APG <jugglinmike> Adam_Page: Yes, that's right <jugglinmike> Matt_King: When Scott presented this, I had the impression that there was something simpler or different about the coding than having to make a button. I guess not <jugglinmike> Adam_Page: Scott made a code pen really early on when he made the issue for APG. I referenced that, taking it an beefing it up <jugglinmike> Adam_Page: I will investigate what's going on with JAWS <jugglinmike> Matt_King: I am using a pre-release version of JAWS, so it may be due to that <jugglinmike> Matt_King: This is really just a different visual design for a disclosure that has a bigger click target <jugglinmike> Adam_Page: Yes, I think that's a fair summary <jugglinmike> Matt_King: Is the issue here about naming and helping people understand that they can have large, rich click targets? <jugglinmike> Matt_King: Maybe we call this a "disclosure region" instead of a "disclosure button" <Daniel> q? <jugglinmike> Adam_Page: Jem called out that there is the Bootstrap "Card". That is a common UX paradigm <jugglinmike> Matt_King: So maybe we call it an "expandable card" <jugglinmike> Adam_Page: That could work <jugglinmike> Matt_King: I think the first step is to move it into the "disclosure" example directory <jugglinmike> Matt_King: Then we can avoid creating a whole new pattern <jugglinmike> Adam_Page: Sounds good <jugglinmike> Matt_King: We ought to be able to get this into the next publication. I'll see if I can mess around with words <Daniel> https://github.com//issues/3215#issuecomment-2626979811 <jugglinmike> ask Daniel <jugglinmike> ack Daniel <Jem> +q <jugglinmike> Daniel: I put a comment for us to think about what we want the screen reader user's experience to be <jugglinmike> Daniel: As you arrow down through the things, you could press "enter" to expand. Making this go to the button and then hitting "enter" on that--maybe we should consider efficiency <jugglinmike> Daniel: And for what it's worth, I don't experience any lag when testing with NVDA (though I'm using an NVDA beta) <jugglinmike> Matt_King: If you're reading this with a screen reader, even if you knew that there was something expandable inside of this region before you got to this button, how would you know that you want to expand it before getting to the button? I'm presuming that you would want to read the content... <jugglinmike> Matt_King: I'm trying to grasp what the alternatives might be and why they might be desirable <jugglinmike> Adam_Page: I have to confess that the "clickable" announcement is a little mysterious to me, still. I don't know if it happens in VoiceOver at all <jugglinmike> Adam_Page: Are JAWS and NVDA trying to be "smart" in the sense that if they see a "click" event listener on an element that is not typically focusable, do they bubble that information up? <jugglinmike> Daniel: NVDA has that behavior enabled by default, but I turn it off <jugglinmike> Matt_King: I do, too <jugglinmike> Daniel: There's a setting in NVDA where you can actually turn it on or off whether you want "clickable" items announced <jugglinmike> Daniel: As Adam_Page was saying, it's whenever they find a "click" event listener <jugglinmike> Jem: Daniel shared that as a comment to issue 3215 <jugglinmike> Matt_King: JAWS and VoiceOver don't announce "clickable" by default, and a lot of NVDA users turn it off <Jem> https://github.com//issues/3215 <jugglinmike> Matt_King: Thank you for the excellent discussion! This is great <jugglinmike> Adam_Page: For my next step, I will do some JAWS testing and update the pull request to make this an example of the disclosure pattern <jugglinmike> Matt_King: Awesome, thank you! <jugglinmike> Zakim, end the meeting |
Regarding my earlier notes about the HTML linter errors, I found two related issues: |
The ARIA Authoring Practices (APG) Task Force just discussed The full IRC log of that discussion<jugglinmike> Topic: PR 3251 - New expandable region pattern<jugglinmike> github: https://github.com//pull/3251 <jugglinmike> Matt_King: Adam added a comment about some linting problems <jugglinmike> Matt_King: It says that there are related issues in the linter. "Allow paragraphs in group" and "false positive for a valid role=image" <jugglinmike> Matt_King: I wonder if those are the two things that are failing here <jugglinmike> Matt_King: It says "some checks not successful", but oh, there's five failing <jugglinmike> Matt_King: The coverage report check is failing. I haven't seen that in a while. How does that happen? <jugglinmike> Matt_King: index.html coverage... Is this index generation or coverage report generation? <jugglinmike> jongund: Looks like someone tried to manually change the coverage report even though it's automatically generated <jugglinmike> Matt_King: "13 files changed"... I honestly forget how we handle this in the build process. It's a little tricky <jugglinmike> jongund: There must have been some manual changes to the coverage file... <jugglinmike> jongund: He could just restore the original file from the "main" branch and see if it builds <jugglinmike> Matt_King: We might want to take lessons learned from modifying the .vnurc file in the patch we considered earlier in this meeting <jugglinmike> Matt_King: If the validator is this buggy, then it almost starts to seem like more trouble than it's worth! I'm mostly joking, though--the truth is, it catches a lot of potential errors |
The ARIA Authoring Practices (APG) Task Force just discussed The full IRC log of that discussion<jugglinmike> topic: PR 3251 - New expandable region pattern<jugglinmike> github: https://github.com//pull/3251 <jugglinmike> Matt_King: This is Adam_Page's awesome pull request <jugglinmike> Matt_King: Before Adam_Page left, we were discussing where to place this example <jugglinmike> Adam_Page: Yes. As I recall, we decided to move it into the existing "disclosure" pattern <jugglinmike> Adam_Page: I made that change. It is now a "disclosure" example. I updated the "patterns" page to link to it <jugglinmike> Matt_King: Do we have an open issue related to our pinning of the version of the linter? <jugglinmike> Matt_King: We had something pinned, if I recall correctly <jugglinmike> howard-e: We do not have an issue, but I created a pull request two weeks ago <jugglinmike> Matt_King: So you have a pull request to un-pin it. I wonder if we should land that before Adam_Page makes any more changes to the linting specific to his pull request <jugglinmike> howard-e: It's pull request 3262 https://github.com//pull/3262 <jugglinmike> Matt_King: That says "all checks have passed" right now on this one <jugglinmike> Matt_King: With this new validator--are the old exceptions still needed? <jugglinmike> howard-e: I didn't confirm, but I can do so pretty quickly <jugglinmike> Matt_King: You can decide and go ahead and merge this as appropriate <jugglinmike> Matt_King: And then after this one is merged, Adam_Page, let's see if your errors still pop up <jugglinmike> Adam_Page: Sounds great! <jugglinmike> howard-e: thank you! <jugglinmike> Adam_Page: I'll stay tuned <jugglinmike> Adam_Page: Ill also check in on NVDA <jugglinmike> s/Ill/I'll/ <jugglinmike> Matt_King: Maybe we should finish the documentation before we seek additional feedback <jugglinmike> Adam_Page: For what it's worth, before I pushed up my initial Pull request months ago, I chatted with Scott, and he endorsed my work <jugglinmike> Matt_King: So you called it "disclosure card"? <jugglinmike> Adam_Page: Yeah, I did, but it's not a strong opinion <jugglinmike> Matt_King: Do you think that we should give it an alternative name in parenthesis, as we do in other places? E.g.. "(expandable region)" <jugglinmike> Adam_Page: Maybe, "Disclosure (show card)" ? <Adam_Page> Disclosure (Show/Hide) Card <jugglinmike> Matt_King: I'm looking at the list of disclosure examples on the patterns page <jugglinmike> Matt_King: In these, the HTML element that is the region--what is the element? Is it a "div" with the "role" region? <jugglinmike> Adam_Page: The HTML element I'm using is "section" and I give it an accname with aria-labeledby <jugglinmike> Adam_Page: That wasn't a strong opinion, either. I just did that because this was an expandable region from the get-go <jugglinmike> Adam_Page: I may have complicated this because I chose to make multiple cards and to place them in a semantic ordered list <jugglinmike> Adam_Page: That seems like a valid use-case, but it may be a distraction for this example <jugglinmike> Matt_King: I guess that if we thought it was really important to show it as a collection of cards, then maybe. But maybe it would be better as an individual <jugglinmike> Adam_Page: Yeah. And cards are often presented as a collection. That's why they're cards. From that perspective, it does feel appropriate to present them as a collection. And if it is a collection, it seems appropriate to present it as a semantic list <jugglinmike> Matt_King: We have two levels of lists here <jugglinmike> Adam_Page: That's because the fiction of the example is a conference with talks spanning multiple days <jugglinmike> Adam_Page: The content is silly--it's tongue-in-cheek that I added hastily. I will swap in more straightforward content <jugglinmike> Matt_King: From the perspective of testing in the ARIA-AT context, it would be good to bring it down to a single list. That would simplify the semantics <jugglinmike> Adam_Page: And I don't think it would limit the example's utility in the APG. I can make that simplification <jugglinmike> Matt_King: This is unlike accordion where you normally have one region exposed. If someone took this and ran with it, they could end up with many regions exposed. <jugglinmike> Matt_King: I think "article" would be a better use. Articles are great for regions--they don't clutter the navigation |
Hey @adampage you can pull the latest vnu validator changes from The pull should also clear up the intermittent link checker errors. For the coverage report error, doing an |
This reverts commit c08ff50.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JS and test changes are good to me! Very clear to me on what's going on. Overall, I'm impressed with the example!
My focus was mainly on the JS and tests for this review but I also left some non-blocking comments inline.
Thanks for all this work!
The ARIA Authoring Practices (APG) Task Force just discussed The full IRC log of that discussion<jugglinmike> Topic: PR 3251 - New example demonstrating expandable cards/regions<jugglinmike> github: https://github.com//pull/3251 <jugglinmike> Matt_King: This is what we've been calling the "expandable region" or the "disclosure card" <jugglinmike> Matt_King: I'm going to finish my review of the editorial content this afternoon <jugglinmike> Matt_King: I know jongund did a code review <jugglinmike> Adam_Page: I've pushed changes for all the feedback I've received so far <jugglinmike> Matt_King: jongund, do you have the time to review again? <jugglinmike> jongund: Sure <jugglinmike> Matt_King: Awesome. I'll request another review from you <jugglinmike> howard-e: I reviewed the tests and some of the code changes. I had no issues <jugglinmike> Matt_King: we had siri and CurtBellew on this one, as well <jugglinmike> Jem: siri isn't here today, but CurtBellew is here <jugglinmike> Matt_King: as I recall, CurtBellew was going to review the design and functionality <jugglinmike> CurtBellew: Yes, I'll do that today <jugglinmike> Matt_King: great, that will help today <jugglinmike> Adam_Page: I made some minor tweaks to the CSS and to the documentation <jugglinmike> Jem: Usually, when we have an example with links, we have a blank page as a placeholder <jugglinmike> Matt_King: The link should do something <jugglinmike> Matt_King: We decided that we weren't going to create a bunch of "dummy" pages <jugglinmike> Matt_King: What do the links in the Tree Grid do? <jugglinmike> Jem: They are "mailto:" links <jugglinmike> Matt_King: There are some links in some of the dialogs. Those might open up a JavaScript alert <jugglinmike> Adam_Page: What does the print button do? <jugglinmike> Jem: It opens a modal dialog <jugglinmike> Matt_King: The probably doesn't impact the tests because we're not testing the keyboard functionality of buttons or links. Only the "expand/collapse" functionality <jugglinmike> Matt_King: We should have a regression test for each keyboard command that's documented <jugglinmike> Adam_Page: Yes, we have that <jugglinmike> Adam_Page: In the CSS, I'm using an experimental feature called "interpolate size". It's purely cosmetic. It is supported by Chrome and Edge, and Firefox may support it behind a flag. <jugglinmike> Matt_King: Does it have a bad side effect if it's not supported? <jugglinmike> Adam_Page: No. It is still functional without it <Jem> https://github.com//pull/3251#discussion_r2155596977 <jugglinmike> Jem: If it doesn't have any bad impact when unsupported, I think we can keep it <jugglinmike> Adam_Page: That sounds fine to me <jugglinmike> Matt_King: I'm aligned with it as well <jugglinmike> howard-e: I'm also fine with it (and I think I said as much in my review) <Jem> https://www.w3.org/WAI/ARIA/apg/practices/read-me-first/ <jugglinmike> howard-e: I wonder if the "read this first" statement covers things like this. In the future, it could enhance the experience if an experimental feature is used, but it could be detrimental if absent <jugglinmike> Jem: I'm sharing the "read this first" page. There, we have a section about "browser and assistive technology support" <jugglinmike> Matt_King: as a code guide practice, I don't know if this is really experimental because it is already spec'd and being implemented. If we had doubt about the feature being broadly supported or if it could have a negative impact on the accessible experience <jugglinmike> Adam_Page: For what it's worth, I gated this behind a "prefers reduced motion" media query <jugglinmike> Matt_King: Great |
Hi @a11ydoer and @mcking65, I found an instance of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the importance of the choice of the article element, I suggest documenting its use in both the accessibility features section and in the roles, states, and properties table.
In accessibility features, I suggest something like:
The content of each card is contained within an HTML
<article>
element that is labeled by the heading element within the card. This gives each card thearticle
role, which enables screen reader users to perceive the boundaries of each card and easily move their reading cursor to the next or previous card. Thearticle
element is preferable to thesection
element becausesection
elements would create ARIA landmark regions, and excessive use of landmark regions diminishes their utility.
In the roles, states, and properties table, I suggest adding a "test not needed" row similar to the row for aria-live=assertive in the
Alert Example.
- This does not have to be declared in the code because it is implicit in the HTML article element.
- Enables screen reader users to perceive the boundaries of each card and easily move a reading cursor from card to card.
…practices into adampage-expandable-region
Thanks, @mcking65. I’ve replaced my previous accessibility feature for I also added the |
I also pushed some changes to the about section to simplify and clarify wording. You may want to double check my work there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Editorial review complete; looking great!
The ARIA Authoring Practices (APG) Task Force just discussed The full IRC log of that discussion<jugglinmike> Topic: PR 3251 - New example demonstrating expandable cards/regions<jugglinmike> github: https://github.com//pull/3251 <jugglinmike> Matt_King: Both Adam_Page and I were pushing changes today. I think those were minor and that we have sufficient approving reviews <jugglinmike> Matt_King: There was one piece of "spin off" feedback regarding the character casing in JavaScript <jugglinmike> Adam_Page: I don't think there is any other feedback that we haven't addressed. <jugglinmike> Adam_Page: There might be some open questions remaining from earlier reviews... Nope, it looks like everything is resolved <Daniel> q+ <jugglinmike> ack Daniel <jugglinmike> Daniel: Do we want to add two phrases for the example to help explain the last paragraph? <jugglinmike> Matt_King: We could make the last paragraph more specific <jugglinmike> Daniel: There's another audience that visit APG to know how things work. And we have pages that are more explanatory for them <jugglinmike> Matt_King: What do you have in mind? Maybe, "Instead, by modifying the structure to be a card that contains a button rather than a heading where all of its content is a button" <jugglinmike> Adam_Page: We're distinguishing it from a vanilla disclosure <jugglinmike> Matt_King: Ah, but the vanilla disclosure doesn't have a heading <jugglinmike> Matt_King: "We modify the disclosure so that instead of the entire disclosure being a button containing plain test, we make it a card with the 'article' role" <jugglinmike> Matt_King: There's a lot of stuff inside of the card besides just the button <jugglinmike> Adam_Page: Right. And that interesting stuff--there's a lot of it in both the "summary" as well as the expanded form <jugglinmike> Matt_King: I'm happy to help doing a little wordsmithing, if you like <jugglinmike> Adam_Page: I can take a whack at it, but I feel like you have a better handle on the essence of what we need to communicate here. If you can offer a suggest, then I can wordsmith it <jugglinmike> Matt_King: Yeah, I can do that. <jugglinmike> Jem: When you search with the "filter", when does this example show up? <jugglinmike> Jem: It's under "disclosure" <jugglinmike> s/Jem:/Matt_King:/ <jugglinmike> Matt_King: We didn't include the words "region" or "expandable" in the final draft because we're not using the "region" role and because the card is more generic <jugglinmike> Matt_King: We don't have a "card" pattern of any kind <jugglinmike> Matt_King: If were to ever add a "card collection" pattern, we would probably used his code as an example of that <jugglinmike> Matt_King: This almost demonstrates a "card collection" pattern. Adam_Page included three cards--that's a collection! <jugglinmike> Matt_King: We have an open issue for search--we never implemented search |
Here is my proposal. Replace the following text:
With the following:
|
Thank you, @mcking65, this reads great to me. I’ve integrated your proposed text into our introduction. |
Thank you. Looks great. It seems like the netlify preview is not building, but it shows up in my local and the preview looks perfect locally. I'll merge now. |
Resolves #3215.
Preview of Example Disclosure (Show/Hide) Card | APG | WAI | W3C
WAI Preview Link (Last built on Tue, 08 Jul 2025 19:56:28 GMT).