-
Notifications
You must be signed in to change notification settings - Fork 28.7k
Adds list and list item roles #164809
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
Adds list and list item roles #164809
Conversation
/// | ||
/// Uses aria list role to convey this semantic information to the element. | ||
/// | ||
/// Screen-readers takes advantage of "aria-label" to describe the visual. |
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.
nit: s/takes/take/
bool focusAsRouteDefault() => focusable?.focusAsRouteDefault() ?? false; | ||
} | ||
|
||
/// Indicates a item in a list. |
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.
nit: s/a item/an item/
/// | ||
/// Uses aria listitem role to convey this semantic information to the element. | ||
/// | ||
/// Screen-readers takes advantage of "aria-label" to describe the visual. |
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.
nit: s/takes/take/
@@ -443,6 +444,12 @@ enum EngineSemanticsRole { | |||
/// A cell in a [table] contains header information for a column. | |||
columnHeader, | |||
|
|||
/// A container that display its children in a list layout. |
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.
nit: s/display/displays/
Instead of saying "list layout" (how is a list laid out? can it be horizontal?), consider saying "...whose children are logically a list of items". We should also probably explain that it is expected that each child node has the role [listItem]
.
/// A container that display its children in a list layout. | ||
list, | ||
|
||
/// An item in a [list]. |
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.
Does listItem
need to be a direct child of a list
? We have a similar situation/problem with tables.
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.
yes, it has the same problem, so I think @hannah-hyj 's pr will help fix this issue
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.
Cool. Do we want to document and/or enforce the relationship in the node structure somehow?
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.
for semanticsnode, yes we have check in framework side. i.e. _semanticsListItem
or do you meant in dom structure?
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.
A check in the framework is probably good enough, and it's probably easier to maintain there.
autosubmit label was removed for flutter/flutter/164809, because - The status or check suite Linux_android_emu android_engine_vulkan_tests has failed. Please fix the issues identified (or deflake) before re-applying this label. |
autosubmit label was removed for flutter/flutter/164809, because Pull request flutter/flutter/164809 is not in a mergeable state. |
fixes #162121
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.