Skip to content

Conversation

@crisbeto
Copy link
Member

Calls preventDefault when handling keyboard events in the ListKeyManager. This avoids the user scrolling the page when using the arrow keys to navigate.

Relates to #1999.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 28, 2016
@crisbeto crisbeto force-pushed the list-key-manager-prevent-default branch from bf641cf to 6868127 Compare November 28, 2016 18:31
this.focusFirstItem();
break;
case END:
event.preventDefault();
Copy link
Member

Choose a reason for hiding this comment

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

Instead of calling preventDefault in each case, you can do:

  onKeydown(event: KeyboardEvent): void {
    switch (event.keyCode) {
      case DOWN_ARROW:
        this.focusNextItem();
        break;
      case UP_ARROW:
        this.focusPreviousItem();
        break;
      case HOME:
        this.focusFirstItem();
        break;
      case END:
        this.focusLastItem();
        break;
      case TAB:
        this._tabOut.next(null);
        return;
      default: 
        return;
    }

    event.preventDefault();
  }

(see slider.ts for an example)

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not supposed to be called when tabbing, though. I initially went with something similar but it broke the menu e2e tests.

Copy link
Member

Choose a reason for hiding this comment

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

That's why the tab case has a return
(it does need a comment to point that out, though)

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated. Also added a unit test for tab, just in case.

@jelbourn
Copy link
Member

R: @kara

Calls preventDefault when handling keyboard events in the ListKeyManager. This avoids the user scrolling the page when using the arrow keys to navigate.

Relates to angular#1999.
@crisbeto crisbeto force-pushed the list-key-manager-prevent-default branch from 6868127 to 55a837d Compare November 29, 2016 18:36
Copy link
Contributor

@kara kara left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added the action: merge The PR is ready for merge by the caretaker label Nov 30, 2016
@tinayuangao tinayuangao merged commit a8355e4 into angular:master Dec 1, 2016
@crisbeto crisbeto mentioned this pull request Dec 1, 2016
3 tasks
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants