-
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
Put buttons in calendar cells #24171
Conversation
work-in-progress, seems to have some layout problems with the table |
Deployed dev-app to: https://ng-comp-dev--pr-24171-5b596d852159e42b1b6e75de27434036-6z6yd80q.web.app |
e867147
to
84d42d3
Compare
Okay, this is working correctly now, so it's ready for review. I've tested it on VoiceOver, NVDA and JAWS with Chrome. |
84d42d3
to
5b4b476
Compare
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.
Could you also add a demo to the demo page with a single date selection with apply/cancel buttons?
[attr.data-mat-row]="rowIndex" | ||
[attr.data-mat-col]="colIndex" |
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.
What are these two attrs for?
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 are used only in our js.
_getCellFromElement
uses them to map a dom element back to the MatCalendarCell
it corresponds to.
https://github.com/angular/components/blob/master/src/material/datepicker/calendar-body.ts#L357
position: absolute; | ||
top: 0; | ||
left: 0; | ||
width: 100%; | ||
height: 100%; |
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 this really need to be position: absolute
?
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.
Well, that was the only way I could find to get that layout to be correct after spending an hour wrangling the CSS. It seemed liked the easiest way to me. We're already using absolute position to layout the other content inside the cell like the.mat-calendar-body-cell-content
and the preview, so it seemed like a reasonable approach to me. I'm open to suggestions :)
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.
@crisbeto the container being display: table-cell
does seem to make this annoying (otherwise flexbox would solve it). I'm rusty here- is this really the best way to make this element 100% width/height?
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.
I think that this is our only option. The problem is that we set padding-top
, padding-bottom
and width
on the table cell which distorts the content.
@jelbourn |
5b4b476
to
6ac50c4
Compare
Ah, I see. In that case everything generally looks good, just want to get Kristiyan's input on the CSS here. |
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
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
6ac50c4
to
ddaebdc
Compare
ddaebdc#diff-c9da88d358126fd9af0bcfe70ca9dce66f53916557502d2e817772d860c9822cR51 |
ddaebdc
to
98ea5bb
Compare
https://github.com/angular/components/pull/24171/files#diff-e546879714a61e2a4f78783bb86bfd5a75f2d1acb0fb1651af43299b12530cacR64 |
a306d9a
to
6cc60a8
Compare
Makes changes to the DOM structure of calendar cells for better screen reader experience. Previously, the DOM structure looksed like this: ``` <!-- Existing DOM structure of each calendar body cell --> <td class="mat-calendar-body-cell" role="gridcell" aria-disabled="false" aria-current="date" aria-selected="true" <!-- ... --> > <!-- additional details ommited --> </> ``` Using the `gridcell` role allows screenreaders to use table specific navigation and some screenreaders would announce that the cells are interactible because of the presence of `aria-selected`. However, some screenreaders did not announce the cells as interactable and treated it the same as a cell in a static table (e.g. VoiceOver announces element type incorrectly angular#23476). This changes the DOM structure to nest buttons inside of a gridcell to make it more explicit that the table cells can be interacted with and are not static content. The gridcell role is still present, so table navigation will continue to work, but the interaction is done with buttons nested inside the `td` elements. The `td` element is only for adding information to the a11y tree and not used for visual purposes. Updated DOM structure: ``` <td role="gridcell" class="mat-calendar-body-cell-container" > <button class="mat-calendar-body-cell" aria-disabled="false" aria-current="date" aria-pressed="true" <!-- ... --> > <!-- additional details ommited --> </button> </td> ``` Fixes angular#23476, angular#24086
6cc60a8
to
5b596d8
Compare
Makes changes to the DOM structure of calendar cells for better screen reader experience. Previously, the DOM structure looksed like this: ``` <!-- Existing DOM structure of each calendar body cell --> <td class="mat-calendar-body-cell" role="gridcell" aria-disabled="false" aria-current="date" aria-selected="true" <!-- ... --> > <!-- additional details ommited --> </> ``` Using the `gridcell` role allows screenreaders to use table specific navigation and some screenreaders would announce that the cells are interactible because of the presence of `aria-selected`. However, some screenreaders did not announce the cells as interactable and treated it the same as a cell in a static table (e.g. VoiceOver announces element type incorrectly #23476). This changes the DOM structure to nest buttons inside of a gridcell to make it more explicit that the table cells can be interacted with and are not static content. The gridcell role is still present, so table navigation will continue to work, but the interaction is done with buttons nested inside the `td` elements. The `td` element is only for adding information to the a11y tree and not used for visual purposes. Updated DOM structure: ``` <td role="gridcell" class="mat-calendar-body-cell-container" > <button class="mat-calendar-body-cell" aria-disabled="false" aria-current="date" aria-pressed="true" <!-- ... --> > <!-- additional details ommited --> </button> </td> ``` Fixes #23476, #24086 (cherry picked from commit 43db844)
Makes changes to the DOM structure of calendar cells for better screen reader experience. Previously, the DOM structure looksed like this: ``` <!-- Existing DOM structure of each calendar body cell --> <td class="mat-calendar-body-cell" role="gridcell" aria-disabled="false" aria-current="date" aria-selected="true" <!-- ... --> > <!-- additional details ommited --> </> ``` Using the `gridcell` role allows screenreaders to use table specific navigation and some screenreaders would announce that the cells are interactible because of the presence of `aria-selected`. However, some screenreaders did not announce the cells as interactable and treated it the same as a cell in a static table (e.g. VoiceOver announces element type incorrectly #23476). This changes the DOM structure to nest buttons inside of a gridcell to make it more explicit that the table cells can be interacted with and are not static content. The gridcell role is still present, so table navigation will continue to work, but the interaction is done with buttons nested inside the `td` elements. The `td` element is only for adding information to the a11y tree and not used for visual purposes. Updated DOM structure: ``` <td role="gridcell" class="mat-calendar-body-cell-container" > <button class="mat-calendar-body-cell" aria-disabled="false" aria-current="date" aria-pressed="true" <!-- ... --> > <!-- additional details ommited --> </button> </td> ``` Fixes #23476, #24086 (cherry picked from commit 43db844)
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/cdk](https://github.com/angular/components) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcdk/13.1.2/13.2.1) | | [@angular/material](https://github.com/angular/components) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fmaterial/13.1.2/13.2.1) | --- ### Release Notes <details> <summary>angular/components</summary> ### [`v13.2.1`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#​1321-vinyl-viola-2022-02-02) [Compare Source](angular/components@13.2.0...13.2.1) ##### cdk | Commit | Type | Description | | -- | -- | -- | | [70d1634e70](angular/components@70d1634) | fix | **a11y:** allow for multiple browser-generated description containers ([#​23507](angular/components#23507)) | ##### material | Commit | Type | Description | | -- | -- | -- | | [d8ddfb04ca](angular/components@d8ddfb0) | fix | **datepicker:** content overflowing when large custom header is provided ([#​24255](angular/components#24255)) | | [d7fe423a3e](angular/components@d7fe423) | fix | **menu:** adjust overlay size when amount of items changes ([#​21457](angular/components#21457)) | | [974d330dc8](angular/components@974d330) | fix | **slider:** Ticks updated wrongly if the max property 0 ([#​24218](angular/components#24218)) | | [a634505190](angular/components@a634505) | fix | **tabs:** use buttons for paginator ([#​14640](angular/components#14640)) | ##### cdk-experimental | Commit | Type | Description | | -- | -- | -- | | [7aff50a6d8](angular/components@7aff50a) | fix | **menu:** keep context menus open when mouse is released ([#​24308](angular/components#24308)) | ##### material-experimental | Commit | Type | Description | | -- | -- | -- | | [c02c43a2b9](angular/components@c02c43a) | fix | **mdc-button:** align outline color with spec ([#​24249](angular/components#24249)) | | [5d7d6ea107](angular/components@5d7d6ea) | perf | **mdc-list:** reduce bundle size ([#​24291](angular/components#24291)) | ##### multiple | Commit | Type | Description | | -- | -- | -- | | [b32d8d1624](angular/components@b32d8d1) | perf | Remove IE 11 cruft from table, column-resize, and popover-edit. ([#​23900](angular/components#23900)) | #### Special Thanks Dmytro Mezhenskyi, Joey Perrott, Karl Seamon, Kristiyan Kostadinov, Miles Malerba, Paul Gschwendtner, Zach Arend and ram <!-- CHANGELOG SPLIT MARKER --> ### [`v13.2.0`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#​1320-terracotta-tiramisu-2022-01-26) [Compare Source](angular/components@13.1.3...13.2.0) ##### material | Commit | Type | Description | | -- | -- | -- | | [b9a3908fcf](angular/components@b9a3908) | feat | **tabs:** add API to update the pagination ([#​23288](angular/components#23288)) | | [f10d245cca](angular/components@f10d245) | feat | **tabs:** label & body classes ([#​23691](angular/components#23691)) | | [ea78a473a1](angular/components@ea78a47) | feat | **tabs:** Refactor MatTabNav to follow the ARIA tabs pattern ([#​24062](angular/components#24062)) | | [337634f899](angular/components@337634f) | fix | **chips:** don't stop propagation on all click events ([#​19763](angular/components#19763)) | | [2b6739742b](angular/components@2b67397) | fix | **datepicker:** change calendar cells to buttons ([#​24171](angular/components#24171)) | | [c55524a8eb](angular/components@c55524a) | fix | **list:** add isDisabled to all list item harnesses ([#​24212](angular/components#24212)) | | [fa7cd154d0](angular/components@fa7cd15) | fix | **list:** fix duplicate focus with chromevox on action-list items ([#​23361](angular/components#23361)) | | [0477022d2c](angular/components@0477022) | fix | **slide-toggle:** remove divs nested inside label ([#​21224](angular/components#21224)) | ##### google-maps | Commit | Type | Description | | -- | -- | -- | | [e6359cdc67](angular/components@e6359cd) | feat | allow for info window focus behavior to be customized ([#​23831](angular/components#23831)) | ##### material-experimental | Commit | Type | Description | | -- | -- | -- | | [c5482c945f](angular/components@c5482c9) | feat | **mdc-chips:** switch to evolution API ([#​23931](angular/components#23931)) | | [723b77ad1f](angular/components@723b77a) | feat | **mdc-core:** add missing color, density, typography mixins ([#​24063](angular/components#24063)) | | [407682012d](angular/components@4076820) | feat | **mdc-form-field:** Add option for dynamic su… ([#​24241](angular/components#24241)) | | [871a500fb8](angular/components@871a500) | feat | **mdc-list:** rework API to support secondary text with wrapping | | [b0f38b7a64](angular/components@b0f38b7) | fix | **mdc-button:** remove unwanted native button styles ([#​24186](angular/components#24186)) | | [c9ab38bcae](angular/components@c9ab38b) | fix | **mdc-chips:** fix changed after checked error when restoring focus to input ([#​24243](angular/components#24243)) | | [68a29ff1dd](angular/components@68a29ff) | fix | **mdc-core:** make mat-option typography easier to override ([#​24247](angular/components#24247)) | | [b79406fee8](angular/components@b79406f) | fix | **mdc-form-field:** Properly handle when defaults setting is 'dynamic' and the subscriptSizing input is not present. ([#​24263](angular/components#24263)) | | [38affc3d43](angular/components@38affc3) | fix | **mdc-list:** ensure selection change event fires properly ([#​24174](angular/components#24174)) | | [c199aa2544](angular/components@c199aa2) | fix | **mdc-list:** incorrect active/hover color for selected items | | [1ce3e5e905](angular/components@1ce3e5e) | perf | **mdc-checkbox:** reduce bundle size ([#​24256](angular/components#24256)) | | [152c60ba12](angular/components@152c60b) | perf | **mdc-radio:** reduce bundle size ([#​24267](angular/components#24267)) | | [02c8f2aa02](angular/components@02c8f2a) | perf | **mdc-tabs:** reduce bundle size ([#​24262](angular/components#24262)) | ##### expansion-panel | Commit | Type | Description | | -- | -- | -- | | [4ec34b5400](angular/components@4ec34b5) | fix | title text not centered with taller description ([#​12161](angular/components#12161)) | #### Special Thanks Amy Sorto, Andrew Seguin, Karl Seamon, Kristiyan Kostadinov, Miles Malerba, Paul Gschwendtner, Ruslan Lekhman, Wagner Maciel, Zach Arend, Zack Elliott, coopermeitz and renovate\[bot] <!-- CHANGELOG SPLIT MARKER --> ### [`v13.1.3`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#​1313-plastic-koala-2022-01-19) [Compare Source](angular/components@13.1.2...13.1.3) ##### cdk | Commit | Type | Description | | -- | -- | -- | | [109d5a150f](angular/components@109d5a1) | fix | **a11y:** not detecting fake mousedown on firefox ([#​23493](angular/components#23493)) | | [c48742eb4e](angular/components@c48742e) | fix | **table:** revert breaking change of CdkTable constructor ([#​24202](angular/components#24202)) | ##### material | Commit | Type | Description | | -- | -- | -- | | [70e0170b95](angular/components@70e0170) | fix | **autocomplete:** don't handle enter events with modifier keys ([#​14717](angular/components#14717)) | | [eae436fdab](angular/components@eae436f) | fix | **autocomplete:** optionSelections not emitting when the list of options changes ([#​14813](angular/components#14813)) | | [402c07b3c7](angular/components@402c07b) | fix | **autocomplete** restore focus after emitting option selected event ([#​18707](angular/components#18707)) | | [761f9f25a8](angular/components@761f9f2) | fix | **card:** handle picture element as mat-card-image ([#​23678](angular/components#23678)) | | [3565cfac59](angular/components@3565cfa) | fix | **core:** make MatOption generic ([#​20242](angular/components#20242)) | | [f0272cf5eb](angular/components@f0272cf) | fix | **core:** throw error if hue does not exist ([#​23612](angular/components#23612)) | | [304afaef1d](angular/components@304afae) | fix | **datepicker:** add focus indication to calendar selected date in high contrast mode ([#​22889](angular/components#22889)) | | [805eee8d07](angular/components@805eee8) | fix | **form-field:** outline gap not recalculated when switching to empty label ([#​23949](angular/components#23949)) | | [feac08f138](angular/components@feac08f) | fix | **input:** inconsistently reading name from input with ngModel ([#​19233](angular/components#19233)) | | [439ad2c59d](angular/components@439ad2c) | fix | **list:** fix up disabled list item styles ([#​18881](angular/components#18881)) | | [4182717c57](angular/components@4182717) | fix | **menu:** not interrupting keyboard events to other overlays ([#​23310](angular/components#23310)) | | [a4f655856e](angular/components@a4f6558) | fix | **paginator:** allow readonly options ([#​24054](angular/components#24054)) | | [966b2c52b7](angular/components@966b2c5) | fix | **progress-bar:** unable to change value through property setter ([#​19025](angular/components#19025)) | | [462cb6d713](angular/components@462cb6d) | fix | **progress-spinner:** animation not working on some zoom levels in Safari ([#​23674](angular/components#23674)) | | [94d466235a](angular/components@94d4662) | fix | **select** component value not in sync with control value on init ([#​18443](angular/components#18443)) | | [ce9d8caa1f](angular/components@ce9d8ca) | fix | **sidenav:** end position sidenav tab order not matching visual order ([#​18101](angular/components#18101)) | | [cb0a2ad940](angular/components@cb0a2ad) | fix | **sidenav** implicit content element being registered twice with scroll dispatcher ([#​13973](angular/components#13973)) | | [7be61b6357](angular/components@7be61b6) | fix | **slider:** avoid error on some touchstart events ([#​23823](angular/components#23823)) | | [81528bc6a1](angular/components@81528bc) | fix | **slider:** first keypress ignored if out-of-bounds value is assigned ([#​23827](angular/components#23827)) | | [64dd8ed8b5](angular/components@64dd8ed) | fix | **slider:** incorrectly inheriting color when nested inside component with theme ([#​21334](angular/components#21334)) | | [99e77829cc](angular/components@99e7782) | fix | **snack-bar:** handle long single-line content ([#​24135](angular/components#24135)) | | [ad21ee20ae](angular/components@ad21ee2) | fix | **table** not clearing some internal references on destroy ([#​16051](angular/components#16051)) | | [9752b1d18f](angular/components@9752b1d) | fix | **table:** better handling of invalid data ([#​18953](angular/components#18953)) | | [e01e579a49](angular/components@e01e579) | fix | **tooltip:** not closing if escape is pressed while trigger isn't focused ([#​14434](angular/components#14434)) | | [4972dc5585](angular/components@4972dc5) | perf | **button:** do not run change detection when the anchor is clicked ([#​23992](angular/components#23992)) | ##### material-experimental | Commit | Type | Description | | -- | -- | -- | | [fe39b55f93](angular/components@fe39b55) | fix | **mdc-checkbox:** emitting fallback values for density CSS variables ([#​24184](angular/components#24184)) | | [0ab3dce58a](angular/components@0ab3dce) | fix | **mdc-snack-bar:** avoid hard reference to base components and align API ([#​21425](angular/components#21425)) | #### Special Thanks Andrew Seguin, Artur Androsovych, Jeri Peier, Joey Perrott, Kristiyan Kostadinov, Paul Gschwendtner and Ruslan Lekhman <!-- CHANGELOG SPLIT MARKER --> </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <[email protected]> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1149 Reviewed-by: crapStone <[email protected]> Co-authored-by: Calciumdibromid Bot <[email protected]> Co-committed-by: Calciumdibromid Bot <[email protected]>
The changes in angular#24171 added a `background: none` to the datepicker cell which broke one of the examples, because it increased the specificity. I don't think that there's a good way to reduce the specificity without moving the entire cell template around again so these changes update the example to be more specific. Fixes angular#24383.
The changes in #24171 added a `background: none` to the datepicker cell which broke one of the examples, because it increased the specificity. I don't think that there's a good way to reduce the specificity without moving the entire cell template around again so these changes update the example to be more specific. Fixes #24383.
The changes in #24171 added a `background: none` to the datepicker cell which broke one of the examples, because it increased the specificity. I don't think that there's a good way to reduce the specificity without moving the entire cell template around again so these changes update the example to be more specific. Fixes #24383. (cherry picked from commit 6cc1833)
The changes in angular#24171 added a `background: none` to the datepicker cell which broke one of the examples, because it increased the specificity. I don't think that there's a good way to reduce the specificity without moving the entire cell template around again so these changes update the example to be more specific. Fixes angular#24383.
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. |
Please see commit message for description of this change.
We can ignore the first commit for the build, that's from another PR. The diff is easier to read with
?w=1
(.e.g. https://github.com/angular/components/pull/24171/files?w=1)