feat(material/select): add page down/up button functionality #25508

forsti0506 · 2022-08-21T14:48:28Z

Hello,

small PR on adding some functionality according to (#3549) and implemented to fit the standard.

Page-Down focuses the last element in the select
Page-Up focus the first element in the select
It's done in open and closed state (https://w3c.github.io/aria-practices/examples/combobox/combobox-select-only.html there it is only in closed state, but for example in Chrome a native select is working cloed and open this way)

Best regards,

Martin

crisbeto · 2022-08-22T06:25:45Z

src/cdk/a11y/key-manager/list-key-manager.ts

@@ -280,6 +282,22 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
          return;
        }

+      case PAGE_UP:
+        if (isModifierAllowed) {
+          this.setFirstItemActive();


Page up/down aren't supposed to focus the first/last items, but rather move one page up/down in the list. The reason why we haven't implemented it in the ListKeyManager is that it doesn't know how large the list is and the offset at which focus is set currently.

Hi, if you take a look at my W3C link, it is possible to go 10 elements down/up or select the first/last option for the implentation of page_up/page_down. Or did I udnerstand somethin wrong from this reference implementation?

The number 10 seems a little arbitrary to me, my understanding is that at least in native applications, it's supposed to go up/down by a page (hence the name).

I definitly understand your concerns. I think the idea behing is, that a combobox is an "popup" which is after opening handled like it is a own page => therefore the jump of 10 (or last/first).

If you like I can implement the jump for 10 items, or we can forget the PR. I only saw it is an easy fix and thought i contribute it :)

I think that's fine, but we should make it configurable, similar to how it's done for the home/end keys. E.g. something with ListKeyManager.withPageUpDown(10).

I pushed some tests and the integration with jumping 10 or to first/last one! Are there sth missing?

crisbeto

The CI is failing, because:

The API goldens need to be updated. You can do it by running yarn approve-api a11y and committing the changed file.
There's a merge commit which is tripping up the commit message validator. Usually we rebase branches to avoid merge commits.

crisbeto · 2022-09-16T07:14:26Z

src/cdk/a11y/key-manager/list-key-manager.ts

+      case PAGE_UP:
+        if (this._pageUpAndDown.enabled && isModifierAllowed) {
+          const targetIndex = this._activeItemIndex - this._pageUpAndDown.delta;
+          this._setActiveItemByIndex(targetIndex > 0 ? targetIndex : 0, 1);


You should be able to simplify this and the PAGE_DOWN handling using the _setActiveItemByDelta method instead.

During writing the code I first had the same intention, but in my opinion this method can only be used to move 1 up or down and not an defined number or did i misunderstand anything?

I see. Let's keep it like this for now.

forsti0506 · 2022-09-16T21:03:07Z

The CI is failing, because:

1. The API goldens need to be updated. You can do it by running `yarn approve-api a11y` and committing the changed file.

2. There's a merge commit which is tripping up the commit message validator. Usually we rebase branches to avoid merge commits.

file is updated
I did the rebase!

crisbeto

LGTM

crisbeto · 2022-09-17T06:25:36Z

src/cdk/a11y/key-manager/list-key-manager.ts

+      case PAGE_UP:
+        if (this._pageUpAndDown.enabled && isModifierAllowed) {
+          const targetIndex = this._activeItemIndex - this._pageUpAndDown.delta;
+          this._setActiveItemByIndex(targetIndex > 0 ? targetIndex : 0, 1);


I see. Let's keep it like this for now.

forsti0506 · 2022-09-19T17:55:42Z

LGTM

Nice, cu soon hopefully :)

angular-automatic-lock-bot · 2022-10-23T00:07:32Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

forsti0506 requested review from crisbeto, jelbourn and devversion as code owners August 21, 2022 14:48

forsti0506 force-pushed the mat-select-Support-PageUp-and-PageDown branch 4 times, most recently from 66cc5a4 to 55f3e82 Compare August 21, 2022 15:29

crisbeto reviewed Aug 22, 2022

View reviewed changes

forsti0506 force-pushed the mat-select-Support-PageUp-and-PageDown branch from 2af210e to 3e1d4f2 Compare August 25, 2022 20:58

forsti0506 requested review from crisbeto and removed request for jelbourn and devversion September 15, 2022 18:14

crisbeto reviewed Sep 16, 2022

View reviewed changes

forsti0506 force-pushed the mat-select-Support-PageUp-and-PageDown branch from 791051e to 24a2143 Compare September 16, 2022 20:20

angular-robot bot added flag: breaking change area: docs Related to the documentation labels Sep 16, 2022

forsti0506 force-pushed the mat-select-Support-PageUp-and-PageDown branch from 1effeda to 24a2143 Compare September 16, 2022 20:53

angular-robot bot removed the flag: breaking change label Sep 16, 2022

forsti0506 force-pushed the mat-select-Support-PageUp-and-PageDown branch from 24a2143 to a50fb0b Compare September 16, 2022 20:56

feat(material/select): add page down/up button functionality

c181fd8

forsti0506 force-pushed the mat-select-Support-PageUp-and-PageDown branch from a50fb0b to c181fd8 Compare September 16, 2022 21:04

crisbeto approved these changes Sep 17, 2022

View reviewed changes

crisbeto changed the title ~~feat(mat/select): add page down/up button functionality~~ feat(material/select): add page down/up button functionality Sep 17, 2022

andrewseguin self-assigned this Sep 22, 2022

andrewseguin merged commit 8ca3155 into angular:main Sep 22, 2022

angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(material/select): add page down/up button functionality #25508

feat(material/select): add page down/up button functionality #25508

forsti0506 commented Aug 21, 2022

crisbeto Aug 22, 2022

forsti0506 Aug 22, 2022

crisbeto Aug 22, 2022

forsti0506 Aug 22, 2022

crisbeto Aug 23, 2022

forsti0506 Aug 26, 2022

crisbeto left a comment

crisbeto Sep 16, 2022

forsti0506 Sep 16, 2022

crisbeto Sep 17, 2022

forsti0506 commented Sep 16, 2022 •

edited

Loading

crisbeto left a comment

crisbeto Sep 17, 2022

forsti0506 commented Sep 19, 2022

angular-automatic-lock-bot bot commented Oct 23, 2022

feat(material/select): add page down/up button functionality #25508

feat(material/select): add page down/up button functionality #25508

Conversation

forsti0506 commented Aug 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crisbeto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forsti0506 commented Sep 16, 2022 • edited Loading

crisbeto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forsti0506 commented Sep 19, 2022

angular-automatic-lock-bot bot commented Oct 23, 2022

forsti0506 commented Sep 16, 2022 •

edited

Loading