-
Notifications
You must be signed in to change notification settings - Fork 390
Add “expandable region” 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
base: main
Are you sure you want to change the base?
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 <[email protected]>
Co-authored-by: Howard Edwards <[email protected]>
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.
@adampage running Weird that lint-staged isn't catching that for you. Seems it should, just glancing at the file changes. |
Sweet! Thanks for all your help, @howard-e. 🙏🏻 I’ve finished resolving all the checks, including an update to |
Hi @mcking65, I’ve finished up with the tweaks we discussed in our last weekly call. I also did some benchtesting with Chrome plus Narrator, NVDA, and JAWS 2024, as well as macOS VoiceOver. All of those combinations seem to behave well. I’m keeping my fingers crossed that the JAWS performance issue you experienced in the previous version won’t be present in the latest commit. 🤞🏻 If it’s still there, please let me know and I’ll investigate that more deeply. |
Resolves #3215.
WAI Preview Link (Last built on Tue, 27 May 2025 21:20:33 GMT).