-
Notifications
You must be signed in to change notification settings - Fork 382
Landmark Regions Practice: Update search landmark guidance to include new HTML search element #3249
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
…de new HTML `search` element The HTML spec now support the `<search>` element. It maps to an implicit `role=search`. Fix: #2652
Landmark Regions Practice: Update `search` landmark guidance to include new HTML `search` element
The ARIA Authoring Practices (APG) Task Force just discussed The full IRC log of that discussion<jugglinmike> Topic: PR 3249 - Add HTML search to landmark practice<jugglinmike> github: https://github.com//pull/3249 <jugglinmike> Matt_King: I created a new pull request and merged an old pull request into that branch <jugglinmike> Matt_King: that's where this pull request originated <jugglinmike> Matt_King: The changes are for the "practices" page for landmarks <jugglinmike> Matt_King: We have information about landmarks in two places onAPG <jugglinmike> Matt_King: In the "featured practices" section and in the "landmarks" pattern <jugglinmike> Matt_King: The "landmarks" pattern is pretty straightforward. It lists all the landmarks that we have (which need to be rewritten because they don't follow the APG template--they are the one aberration we have left... but that's a separate piece of work) <jugglinmike> Matt_King: My question is: do we want to make a change to the practice without updating the corresponding "search" example page? <jugglinmike> Matt_King: In the first comment on this pull request, I added a link to the preview that goes to the right place <jugglinmike> Matt_King: The first change is under the heading "HTML Sectioning Elements" <jugglinmike> Matt_King: The very last row of that table is new. It's saying that there is now an html "search" element and it corresponds to the role <jugglinmike> Matt_King: The other change is under the section simply titled "Search" (a level-three heading) <jugglinmike> Matt_King: Under that, there is a change under "HTML technique". There is also an "ARIA technique" <jugglinmike> Matt_King: That's essentially all that this PR changes--those two places <jugglinmike> Matt_King: A little further down, you'll find a link to two examples. It says "search landmark examples." If you follow that link, the page has a tab called "HTML techniques". It says that there is no "search" element. This pull request did not change that. <jugglinmike> Matt_King: Should we push this pull request without updating that part? <jugglinmike> Matt_King: Because this landmark example page needs to be rewritten (like all landmark example pages) <jugglinmike> Matt_King: But if we pushed this pull request as-is, people would get conflicting information <jugglinmike> Matt_King: So I'm reluctant to do that <jugglinmike> Jem: I would support to push changes for search and then include a note on this example page that references your new text <jugglinmike> Matt_King: I don't know about a note... I think we would just want to change the content of what's under the "HTML Techniques" tab <jugglinmike> Matt_King: It's just a content update to this page. I didn't have time to make that change to that page <jugglinmike> Matt_King: We do have the option to push this pull request out without changing it, but then the APG would have conflicted information <jugglinmike> Matt_King: Ideally, we want an HTML version of the example under the "ARIA Techniques" tab <jugglinmike> Matt_King: It's pretty straightforward. I'm recommending that it be done before we push this out <jugglinmike> Matt_King: But that means someone needs to add a commit to pull request 3249 which does that <jugglinmike> CurtBellew: This may be something that I actually have time to do <jugglinmike> CurtBellew: It would look almost exactly the same as the ARIA Technique <jugglinmike> Matt_King: Right, I think all you have to do is change the "div" to the "search" element and take away the "role=search". Oh, and also modify the text that's above it <jugglinmike> CurtBellew: Got it <Daniel> q+ <jugglinmike> Matt_King: That would be really helpful! <Daniel> q+ <jugglinmike> Jem: I assigned the pull request to CurtBellew <jugglinmike> ack Daniel <Daniel> https://github.com//issues/3250 -> Issue on respec links <jugglinmike> Daniel: Unrelatedly, there are ReSpec links in the first part of this example, and those no longer work. We should probably audit all the examples for this and fix it throughout with a separate patch <jugglinmike> q? <jugglinmike> Matt_King: I didn't recognize that as being ReSpec. It looks like Wiki markup. I will fix that in this pull request <jugglinmike> Daniel: I'll take a pass on the rest of the pages and see if it shows up elsewhere <jugglinmike> Matt_King: Thank you for that observation, Daniel! |
The ARIA Authoring Practices (APG) Task Force just discussed The full IRC log of that discussion<jugglinmike> Topic: PR 3249 - Add HTML search to landmark practice<jugglinmike> github: https://github.com//pull/3249 <jugglinmike> Matt_King: jongund, I saw you pushed some additional commits <jugglinmike> Matt_King: After we're done with the "color settings" patch, I think we should talk about what we need to do in the "landmark" section overall <jugglinmike> Matt_King: That said, I think the direction #3216 is consistent with the way we've structured the rest of APG <jugglinmike> Matt_King: Instead of pulling stuff from the examples into the "patterns" page, if there is additional information, it should go to the "practices" page <jugglinmike> Matt_King: The "patterns" page was intentionally very simple and referred to the "practices" page <jugglinmike> Matt_King: I'm thinking that we can keep #3216 "on hold" for now, and we can talk about it in a later meeting <jugglinmike> Matt_King: In the mean time, I did want to land #3249 because it's a very simple change and it came from a first-time contributor <jugglinmike> Matt_King: CurtBellew was going to add another simple commit to that to adjust the example page for the "search" landmark <jugglinmike> Matt_King: I'm kind of wondering: if you already did some of the work that CurtBellew was going to do (I saw you push a commit that was similar)--did you make a change to the "search example" landmark page? <jugglinmike> jongund: Yes <jugglinmike> Matt_King: Okay, so CurtBellew, you might want to look at that commit <jugglinmike> jongund: I added a demonstration of an "HTML way" to create a "search" landmark <jugglinmike> CurtBellew: That sounds like exactly what I had done <jugglinmike> Matt_King: I didn't see you push a commit to #3249, yet <jugglinmike> CurtBellew: I've never pushed a commit to an existing pull request, so I had questions about how to do that <Jem> https://github.com/w3c/aria-practices/wiki/Submitting-Pull-Requests <jugglinmike> Matt_King: [explains the process] <jugglinmike> CurtBellew: That makes sense to me! <jugglinmike> Matt_King: Great <jugglinmike> Matt_King: Now, I pushed some more changes to this branch last Wednesday, and my changes still aren't showing up in the preview... <jugglinmike> howard-e: It says it was last built on Wednesday <jugglinmike> Matt_King: I have commit ed14fc6. That commit is not showing up in the preview <jugglinmike> howard-e: I will check that out in the pull request <jugglinmike> s/ed14/eb14/ <jugglinmike> Matt_King: Ok, great <jugglinmike> Matt_King: That's it for today. Thanks, everyone! <jugglinmike> Zakim, end the meeting |
@mcking65 following up from today's TF meeting -- this needed a cache refresh on Netlify. Both yours and @curtbellew's latest contributions are now present at: The cache refresh need has been happening more often so there is something to investigate further here. |
I pushed a commit to this branch, and it is not showing in the preview. |
Following up from earlier sync discussion: A re-run on Netlify got this updated. It seems some Netlify behavior may have changed when it comes to git submodules (wai-aria-practices building aria-practices). Actively investigating on how to make that more robust. Tracking this particular failure at w3c/wai-aria-practices#398 (comment). |
The ARIA Authoring Practices (APG) Task Force just discussed The full IRC log of that discussion<jugglinmike> Topic: PR 3249 - Add HTML search to landmark practice<jugglinmike> github: https://github.com//pull/3249 <jugglinmike> Matt_King: I tried to push a commit to this, and my commit wasn't getting represented... <jugglinmike> howard-e: We fixed that <jugglinmike> Matt_King: Okay, then I think this is ready for review <jugglinmike> Matt_King: There's a link to the "search landmark" example, and CurtBellew's pull request changes that by adding an "HTML technique" tab <jugglinmike> Matt_King: And yes, I see my change here, now <jugglinmike> Adam_Page: I'll be a reviewer <jugglinmike> Matt_King: Thanks! <jugglinmike> Adam_Page: We moved the conditions--why? <jugglinmike> Matt_King: Because it was kind of backwards. If you have a footer that meets these conditions, that's when it maps to "content info". It's not the other way around. Footer has to be used in a certain way... <jugglinmike> Adam_Page: Right, but I still associated the condition with the landmark role <jugglinmike> Matt_King: Exactly <jugglinmike> Adam_Page: So it makes more sense to me to attach the condition to the element rather than the role that it becomes <jugglinmike> Matt_King: So, in that table, for "footer", it says it maps to content info but only if the footer is in the context of a body element <jugglinmike> Matt_King: I guess we could change the wording. "content info if the footer is in," etc. And then we could say, "otherwise, it's not mapped" or "otherwise, it's generic" <Adam_Page> https://www.w3.org/WAI/ARIA/apg/patterns/landmarks/examples/HTML5.html <jugglinmike> Matt_King: It can be done either way, but the way it was worded didn't make sense to me. We could change the wording and do it the other way around, though <jugglinmike> Adam_Page: We obviously have the same understanding of reality. I think this is just about the editorial trade-offs. Let me give this a more thorough review <jugglinmike> Matt_King: We're assigning Adam_Page to review this, and I think that will be enough reviewers (this is mostly editorial) <jugglinmike> Matt_King: I'm open to changing the conditions <jugglinmike> Zakim, end the meeting |
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.
Heya @mcking65, this all looks good to me.
After reviewing it all the way through, I would still editorially prefer to pair that qualifying condition — “if it is in the context of a body element” — with the the contentinfo
role rather than the footer
HTML element, similar to how it’s expressed on this page.
The ARIA Authoring Practices (APG) Task Force just discussed The full IRC log of that discussion<jugglinmike> Topic: PR 3249 - Add HTML search to landmark practice<jugglinmike> github: https://github.com//pull/3249 <jugglinmike> Matt_King: It appears that there are lint errors <jugglinmike> Matt_King: Can someone here look into the errors and maybe push a commit to fix them? I think that is the only thing standing in the way of merge <jugglinmike> howard-e: The error reads, "Element 'search' is not allowed as a child of element 'div' in this context" <jugglinmike> Matt_King: That's interesting. Why would that not be allowed? Maybe we need to ignore it <jugglinmike> Matt_King: I suppose this requires some investigation <jugglinmike> Matt_King: There must be something about the content-model for search... <jugglinmike> howard-e: I guess if it comes down to ignoring it, I can push a commit to do that. <jugglinmike> Matt_King: But it seems strange that we would want to ignore this error if we aren't using the "search" element correctly <jugglinmike> Adam_Page: I can look into this one. You can assign it to me <jugglinmike> Matt_King: Thanks! <jugglinmike> howard-e: The validator is out of date. We pinned to an older version when they released a change that wasn't compatible with our project <jugglinmike> Matt_King: It's very possible that this lint error is because our validator is out of date <jugglinmike> Matt_King: Could we experiment in a pull request? <jugglinmike> howard-e: Yes <jugglinmike> howard-e: All of these errors are isolated to the tree grid, the dialog modal, and the combobox <jugglinmike> howard-e: I'll do some investigation asynchronously |
Heya @mcking65 and @howard-e, the HTML spec’s own Turn out this is a known issue in the validator itself: validator/validator#1569, and there’s an open PR for it. |
Continue work started by @ramiy in #2923 to resolve issue #2652.
Preview link
View landmark practice page in compare branch
WAI Preview Link (Last built on Tue, 08 Apr 2025 14:04:49 GMT).