-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
fix(expansion-panel): implement keyboard controls #12427
Conversation
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.
LGTM, @josephperrott should also take a look
src/lib/expansion/expansion-panel.ts
Outdated
* Base interface for a `MatAccordion`. | ||
* @docs-private | ||
*/ | ||
export interface MatAccordionBase extends CdkAccordion { |
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.
Shouldn't MatAccordionDisplayMode
, MatAccordionBase
and MAT_ACCORDION
be defined in the accordion.ts
file rather than the expansion-panel.ts
file?
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.
They should, but then we'd get a circular import because accordion.ts
imports expansion-panel.ts
.
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.
Can we moved it to another file then?
I think the discovery of the type and interface doesn't make sense since they are for accordion. We could just have an accordion-base.ts
file?
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.
Done.
f17dd4a
to
4f12641
Compare
Hi @crisbeto! This PR has merge conflicts due to recent upstream merges. |
Based on the [accessibility guidelines](https://www.w3.org/TR/wai-aria-practices-1.1/#accordion), accordions should be able to support moving focus using the keyboard. These changes implement the keyboard support and move some things around to avoid circular imports.
4f12641
to
59e4bc3
Compare
Based on the [accessibility guidelines](https://www.w3.org/TR/wai-aria-practices-1.1/#accordion), accordions should be able to support moving focus using the keyboard. These changes implement the keyboard support and move some things around to avoid circular imports.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Based on the accessibility guidelines, accordions should be able to support moving focus using the keyboard. These changes implement the keyboard support and move some things around to avoid circular imports.