Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings
/ariaPublic

Add command & commandfor attribute related mappings#2354

Merged
scottaohara merged 32 commits intomainfrom
add-commandfor-attribute-related-mappings
Apr 2, 2025
Merged

Add command & commandfor attribute related mappings#2354
scottaohara merged 32 commits intomainfrom
add-commandfor-attribute-related-mappings

Conversation

@keithamus
Copy link
Member

@keithamuskeithamus commentedOct 11, 2024
edited
Loading

This adds the mapping forcommand andcommandfor attributes as per theInvokers Explainer.

Additions:

Test, Documentation and Implementation tracking

Once this PR has been reviewed and has consensus from the working group, tests should be written and issues should be opened on browsers. Add N/A and check when not applicable.

WebKit internal tests:https://searchfox.org/wubkat/source/LayoutTests/accessibility/mac/expanded-notification-commandfor-popover.html

Chrome internal tests:https://source.chromium.org/chromium/chromium/src/+/main:content/test/data/accessibility/html/commandfor-api-popover.html;l=1?q=commandfor.*api&sq= &https://source.chromium.org/chromium/chromium/src/+/main:content/test/data/accessibility/html/commandfor-api-dialog.html


Preview |Diff

@netlify
Copy link

netlifybot commentedOct 11, 2024
edited
Loading

Deploy Preview forwai-aria ready!

NameLink
🔨 Latest commit956a062
🔍 Latest deploy loghttps://app.netlify.com/sites/wai-aria/deploys/67ed49fd5315090008a88040
😎 Deploy Previewhttps://deploy-preview-2354--wai-aria.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to yourNetlify site configuration.

Copy link
Member

@scottaoharascottaohara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

feel free to reach out if you have questions / want to talk through what i was suggesting

@keithamus
Copy link
MemberAuthor

Okay@scottaohara I've expanded it as you described, I think it makes a lot more sense. Now each state has its own table which prescibes how that state should map. I've included all currently specified states, toggle/show/hide popover, close, and show modal.

scottaohara reacted with heart emoji

@keithamuskeithamusforce-pushed theadd-commandfor-attribute-related-mappings branch from345eeda to0b1053cCompareOctober 17, 2024 22:29
@scottaohara
Copy link
Member

ok i think this can work.

we still need attribute tables for command and command for. but as far as i'm concerned, those attribute tables could be simplified to just list the attribute, and point to the instances you made in the element section.

e.g., like how thetype attribute for inputs is listed - but simpler... i could see that just removing all the individual mapping rows and in the comments, just link to the first of the button (command in the x state) tables you made.

(mental note to reduce redundancy and unnecessary page link by squashing all "not mapped" or "see ..." rows into a single comment row for other attributes too...)

rahimabdi reacted with thumbs up emoji

@annevk
Copy link
Member

whatwg/html#9841 is close to ready. Is this expected to be merged soon@scottaohara &@aleventhal? Ideally we can merge both around the same time.

taking aaron's feedback, re-combined the popover commands into a single table, since they all should match.similarly, combined the close and show-modal into a single table - as these also should have matched.  this means i removed all the extra aria-expanded mappings that were in the close command.  but we dont need these.  a close button won't exist outside of a modal dialog, since it'd be inert.  and we don't need expanded states on the close button, because someone is already within the dialog.
moves the command tables to the attributes section.  otherwise, a new table for command would have been necessary, and it would have then just pointed to these other tables.  so, moving them down here negates the need for that. adds the commandfor attribute mapping table
@scottaoharascottaohara self-requested a reviewDecember 10, 2024 19:00
@scottaohara
Copy link
Member

@keithamus i have taken some of aaron's feedback and reconfigured this a bit.

The related states are now their own mapping tables popover (toggle,show,hide) and dialog (close, show-modal). These have been moved to the attribute section, rather than in the element section. This was more just a partial re-consolidation and moving around, where the only major change I made was removed the mappings for aria-expanded for the close command. Exposing that state should have been similar to a close button within a popover, where the expanded state isnt' necessary to communicate in that context.

i added the missingcommandfor table - but it's largely there to indicate that it serves as a way to associate the button and the element it commands - but again the mappings under thecommand states are where any programmatic associations that need to be exposed to users are defined.

cc@aleventhal@jcsteh@cookiecrook /@rahimabdi for reviews for your platforms - but not much should be different here since these mappings were already similarly specified forpopover/popovertarget.

I'm particularly curious if Webkit is looking to implement any of the aria-details relationships that chrome/firefox have implemented, and if anything more specific needs to be added to the ax row of the command=show,hide,toggle (or popovertarget for that matter)

@scottaoharascottaohara self-requested a reviewDecember 10, 2024 19:54
@scottaohara
Copy link
Member

@annevk pending reviews, i think@keithamus did a good job getting this setup, and i spent parts of today finishing it up.
pinged reviewers in my previous comment.

@cookiecrookcookiecrook self-requested a reviewDecember 12, 2024 22:54
webkit-commit-queue pushed a commit to lukewarlow/WebKit that referenced this pull requestMar 13, 2025
https://bugs.webkit.org/show_bug.cgi?id=269672Reviewed by Tyler Wilcock and Darin Adler.This patch updates WebKit to handle exposing the expanded state on command buttons.Specifically when the command button is linked with a popover,and has a valid command for popover it will expose the expanded state.Seew3c/aria#2354* LayoutTests/accessibility/mac/expanded-notification-commandfor-popover-expected.txt: Added.* LayoutTests/accessibility/mac/expanded-notification-commandfor-popover.html: Added.* Source/WebCore/accessibility/AXObjectCache.cpp:(WebCore::AXObjectCache::handleAttributeChange):(WebCore::AXObjectCache::updateIsolatedTree):(WebCore::AXObjectCache::relationAttributes):(WebCore::AXObjectCache::attributeToRelationType):(WebCore::AXObjectCache::updateRelations):* Source/WebCore/accessibility/AXObjectCache.h:* Source/WebCore/accessibility/AccessibilityNodeObject.cpp:(WebCore::AccessibilityNodeObject::commandForElement const):(WebCore::AccessibilityNodeObject::commandType const):* Source/WebCore/accessibility/AccessibilityNodeObject.h:* Source/WebCore/accessibility/AccessibilityObject.cpp:(WebCore::AccessibilityObject::commandType const):(WebCore::AccessibilityObject::supportsExpanded const):(WebCore::AccessibilityObject::isExpanded const):* Source/WebCore/accessibility/AccessibilityObject.h:(WebCore::AccessibilityObject::commandForElement const):* Source/WebCore/dom/Element.cpp:(WebCore::Element::popoverState const):* Source/WebCore/dom/Element.h:* Source/WebCore/html/HTMLButtonElement.h:* Source/WebCore/html/HTMLElement.cpp:(WebCore::HTMLElement::popoverState const): Deleted.* Source/WebCore/html/HTMLElement.h:Canonical link:https://commits.webkit.org/292067@main
Co-authored-by: James Craig <cookiecrook@users.noreply.github.com>
@scottaohara
Copy link
Member

As this has landed in the html spec, is shipped in chrome/edge and is being implemented by other browsers, i'm merging this PR.

@scottaoharascottaohara merged commitde2ce16 intomainApr 2, 2025
7 checks passed
@scottaoharascottaohara deleted the add-commandfor-attribute-related-mappings branchApril 2, 2025 14:39
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@scottaoharascottaoharascottaohara approved these changes

@cookiecrookcookiecrookcookiecrook approved these changes

@pkrapkrapkra approved these changes

@smhigleysmhigleyAwaiting requested review from smhigley

@aleventhalaleventhalAwaiting requested review from aleventhal

+1 more reviewer

@lukewarlowlukewarlowlukewarlow left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@keithamus@scottaohara@annevk@lukewarlow@cookiecrook@pkra@aleventhal

Comments


[8]ページ先頭

©2009-2026 Movatter.jp