Skip to content
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

autocomplete - HOME and END keys not working #3496

Closed
mxii opened this issue Mar 8, 2017 · 8 comments · Fixed by #4544
Closed

autocomplete - HOME and END keys not working #3496

mxii opened this issue Mar 8, 2017 · 8 comments · Fixed by #4544
Assignees
Labels
Accessibility This issue is related to accessibility (a11y) help wanted The team would appreciate a PR from the community to address this issue P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@mxii
Copy link

mxii commented Mar 8, 2017

Bug, feature request, or proposal:

I would like to use those keys as "normal" to navigate through my input string..
Like "shift + POS1" will select all.. :)

What is the expected behavior?

Maybe behavior of those keys could be configured?!

What is the current behavior?

Key will only be used to navigate through autocomplete-results

What are the steps to reproduce?

.. should be clear ..

What is the use-case or motivation for changing an existing behavior?

Better usability.
If i want to clear an input field, i will always press "shift + pos1" .. but now its not working anymore! =/
I would guess I am not the only one doing it like this..

Which versions of Angular, Material, OS, browsers are affected?

angular4, material.beta2

Is there anything else we should know?

No, thanks! :)

@crisbeto
Copy link
Member

crisbeto commented Mar 8, 2017

For reference, POS1 is the Home key on German keyboards.

@jefersonestevo
Copy link
Contributor

+1

As angularjs autocomplete does allow to use these keys on the input, I agree that this should work on this component too.

@crisbeto crisbeto assigned crisbeto and unassigned crisbeto Mar 21, 2017
crisbeto added a commit to crisbeto/material2 that referenced this issue Mar 21, 2017
* Fixes the autocomplete not scrolling to active items that were highlighted by pressing `home` or `end`.
* Switches up the logic to scroll to any newly-focused options. This should make it more future-proof, if we add more shortcuts to the key manager.

Relates to angular#3496.
@jelbourn jelbourn added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent help wanted The team would appreciate a PR from the community to address this issue labels Apr 9, 2017
@jefersonestevo
Copy link
Contributor

@crisbeto @jelbourn I was looking to implement this, and I think the implementation here should consider the following paths:

  • If the user press Shift+Home or Shift+End, it will select the whole text, as a normal input field does
  • If there are only one or none option available, the Home and End key should behave like the input text field (changing the cursor to the beggining and end of the input field)

Both of these behaviours can be achieved by some conditional tests on the onKeydown method of ListKeyManager, but as this component is used in multiple components, If I put some code in it, it may break some other components behaviour. What may be done here is to add a second parameter (optional) to the onKeydown method to "decide" if the event.preventDefault() should be called or not. But I don't really know if it's the best way to implement this.

Example (just a simple conditional that may be extracted to a callback):

  onKeydown(event: KeyboardEvent): void {
    let shouldPreventDefault: boolean = true;
    switch (event.keyCode) {
      case DOWN_ARROW:
        this.setNextItemActive();
        break;
      case UP_ARROW:
        this.setPreviousItemActive();
        break;
      case HOME:
        this.setFirstItemActive();
        shouldPreventDefault = !event.shiftKey && this._items.length > 1;
        break;
      case END:
        this.setLastItemActive();
        shouldPreventDefault = !event.shiftKey && this._items.length > 1;
        break;
      case TAB:
        // Note that we shouldn't prevent the default action on tab.
        this._tabOut.next(null);
        return;
      default:
        return;
    }

    if (shouldPreventDefault) {
      event.preventDefault();
    }
  }

It'll really be nice if you can offer some guidance on the best way to implement this.

@jelbourn jelbourn changed the title autocomplete - POS1 and END keys not working autocomplete - HOME and END keys not working Apr 24, 2017
@jonbrock
Copy link

I've also been looking at this. There is some special handling done in autocomplete-trigger.ts already, so I added a check to see if the panel is actually open and if the shift key is being pressed. In my testing this works pretty intuitively. However, I'm not sure what the desired behavior is if home or end is pressed while the panel is closed.

  _handleKeydown(event: KeyboardEvent): void {
    if (this.activeOption && event.keyCode === ENTER) {
      this.activeOption._selectViaInteraction();
      event.preventDefault();
    } else {
      const prevActiveItem = this.autocomplete._keyManager.activeItem;
      const isArrowKey = event.keyCode === UP_ARROW || event.keyCode === DOWN_ARROW;

      if (isArrowKey) {
        this.openPanel();
      }
      
      // Only intercept keys if the panel is open and the shift key is not being used.
      if (this.panelOpen && !event.shiftKey) {
        this.autocomplete._keyManager.onKeydown(event);
      }

      Promise.resolve().then(() => {
        if (isArrowKey || this.autocomplete._keyManager.activeItem !== prevActiveItem) {
          this._scrollToOption();
        }
      });
    }
  }

@wulfsberg
Copy link

wulfsberg commented Apr 24, 2017

Please don't hijack the HOME and END with any special rules in special cases; simply let them work as they do in normal text editing.
If you are dead set on making special rules in some cases, please allow for a configurable opt-out.

I am very used to going back and editing input fields in complex searches, and having that sometimes be "randomly" blocked would be very surprising and frustrating.

@jefersonestevo
Copy link
Contributor

@jonbrock your solution is really good and simple.

The only point that I can see that maybe be a problem is that the ListKeyManager also handles the TAB key. If the user press the Shift+TAB key (while the panel is open), the autocomplete-trigger will not fire the panelClosingActions event and someone that listen to this event will not be notified that the panel has been closed.

Of course we can handle this with:

     if (event.keyCode === TAB || (this.panelOpen && !event.shiftKey)) {
        this.autocomplete._keyManager.onKeydown(event);
      }

But I don't know if it's a good idea to the autocomplete-trigger to know that much of the implementation of the ListKeyManager (If the ListKeyManager implementation changes we may need to change the autocomplete-trigger implementation too).

If the material team think this is ok we can totally go this way.

@mmalerba mmalerba added the Accessibility This issue is related to accessibility (a11y) label Apr 26, 2017
@shaunak1111
Copy link

What is the default behavior for safari for HOME and END keys? In the angular1 material it is the default behavior as intended for input fields. The browserstack tests are failing for HOME and END keys, therefore their behavior on SAFARI for IOS needs discussion

@crisbeto crisbeto self-assigned this May 14, 2017
crisbeto added a commit to crisbeto/material2 that referenced this issue May 14, 2017
… is interacting with the element

* Refactors the `ListKeyManager` to only handle the home and end keys after the user has interacted. A list becomes interactive if the previous keypress, that occurred inside it, was handled by the key manager. This allows us to continue supporting skipping to the first/last items with home/end, while also not blocking the default behavior in cases like the select where it can be useful when editing text.
* Reworks the Autocomplete, ListKeyManager and ChipList tests to use the `createKeyboardEvent`, instead of making their own fake keyboard events.
* Adds a `target` parameter to the `createKeyboardEvent` function, allowing us to define the event target.

Fixes angular#3496.
crisbeto added a commit to crisbeto/material2 that referenced this issue May 16, 2017
* Removes the handling for the home and end keys from the ListKeyManager since their usage isn't as generic as the arrow keys.
* Adds the home and end key handling to the select specifically.
* Reworks the Autocomplete, ListKeyManager and ChipList tests to use the `createKeyboardEvent`, instead of making their own fake keyboard events.
* Adds a `target` parameter to the `createKeyboardEvent` function, allowing us to define the event target.

Fixes angular#3496.
crisbeto added a commit to crisbeto/material2 that referenced this issue May 18, 2017
* Removes the handling for the home and end keys from the ListKeyManager since their usage isn't as generic as the arrow keys.
* Adds the home and end key handling to the select specifically.
* Reworks the Autocomplete, ListKeyManager and ChipList tests to use the `createKeyboardEvent`, instead of making their own fake keyboard events.
* Adds a `target` parameter to the `createKeyboardEvent` function, allowing us to define the event target.

Fixes angular#3496.
crisbeto added a commit to crisbeto/material2 that referenced this issue May 20, 2017
* Removes the handling for the home and end keys from the ListKeyManager since their usage isn't as generic as the arrow keys.
* Adds the home and end key handling to the select specifically.
* Reworks the Autocomplete, ListKeyManager and ChipList tests to use the `createKeyboardEvent`, instead of making their own fake keyboard events.
* Adds a `target` parameter to the `createKeyboardEvent` function, allowing us to define the event target.

Fixes angular#3496.
tinayuangao pushed a commit that referenced this issue May 22, 2017
* Removes the handling for the home and end keys from the ListKeyManager since their usage isn't as generic as the arrow keys.
* Adds the home and end key handling to the select specifically.
* Reworks the Autocomplete, ListKeyManager and ChipList tests to use the `createKeyboardEvent`, instead of making their own fake keyboard events.
* Adds a `target` parameter to the `createKeyboardEvent` function, allowing us to define the event target.

Fixes #3496.
@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 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) help wanted The team would appreciate a PR from the community to address this issue P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
8 participants