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

Next closest chip is not focused after chip removal. #12374

Closed
ghost opened this issue Jul 25, 2018 · 1 comment · Fixed by #12416
Closed

Next closest chip is not focused after chip removal. #12374

ghost opened this issue Jul 25, 2018 · 1 comment · Fixed by #12416
Assignees
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@ghost
Copy link

ghost commented Jul 25, 2018

Bug, feature request, or proposal:

Next closest chip is not focused after chip removal.

What is the expected behavior?

Backspace should delete one chip per keystroke. Specifically, after chip is deleted, the next closest chip should be given focus so the backspace keystroke can target it.

What is the current behavior?

Focus is given to body on chip removal. User must select a chip before each backspace keystroke.

What are the steps to reproduce?

Open https://stackblitz.com/edit/angular-gbfa3u?file=app/chips-input-example.ts in chrome. (Edge and Firefox work ok)
select the last chip
hit backspace (last chip is removed, focus is lost)
hit backspace again - nothing happens

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

Allow animations to be used in a component containing chips without breaking backspace workflow.

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

Angular 6.0.9
Material 6.4.1
Windows 10
Typescript 2.7.2
Chrome version 67.0.3396.99 (Official Build) (64-bit)

Is there anything else we should know?

StackBlitz is a fork of material.angular.io's "Chips with input" sample. Animation definition was added:
animations: [
trigger(
'oeuoeuoeu', [
transition(':leave', [
style({opacity: 0}),
animate('500ms', style({opacity: 1}))
])
]
)
],

'oeuoeuoeu' is simply defined and not attached to anything. Defining an animation, causes the chips to be tagged with a namespace. On removal, when the namespace is defined, ns.removeNode it triggered and the chip disappears from the DOM:

TransitionAnimationEngine.prototype.removeNode = function (namespaceId, element, context) {
if (!isElementNode(element)) {
this._onRemovalComplete(element, context);
return;
}
var ns = namespaceId ? this._fetchNamespace(namespaceId) : null;
if (ns) {
ns.removeNode(element, context);
}
else {
this.markElementAsRemoved(namespaceId, element, false, context);
}

After the chip is removed, _lastDestroyedIndex is null so the next closest chip is not focused:
MatChipList.prototype._updateFocusForDestroyedChips = /**
* Checks to see if a focus chip was recently destroyed so that we can refocus the next closest
* one.
* @return {?}
/
function () {
var /
* @type {?} / chipsArray = this.chips.toArray();
if (this._lastDestroyedIndex != null && chipsArray.length > 0 && (this.focused ||
(this._keyManager.activeItem && chipsArray.indexOf(this._keyManager.activeItem) === -1))) {
// Check whether the destroyed chip was the last item
var /
* @type {?} / newFocusIndex = Math.min(this._lastDestroyedIndex, chipsArray.length - 1);
this._keyManager.setActiveItem(newFocusIndex);
var /
* @type {?} */ focusChip = this._keyManager.activeItem;
// Focus the chip
if (focusChip) {
focusChip.focus();
}
}
// Reset our destroyed index
this._lastDestroyedIndex = null;
};

When animations are removed, chips are not provided with a namespace and thus markElementAsRemoved is triggered instead. The chip lives in the DOM until after _updateFocusForDestroyedChips executes and successfully focuses the next closest chip.

Note:
IE and Firefox do successfully trigger ns.removeNode(element, context), remove the element, and correctly focus the next chip. This is not accomplished by _updateFocusForDestroyedChips though. It still fails because _lastDestroyedIndex is still null. Some other workflow in the angular machinery is refocusing the chip.

@crisbeto crisbeto self-assigned this Jul 25, 2018
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 28, 2018
…nside component with animations

Fixes the chip list losing its focus position if a chip is deleted while it's inside a component with animations.

Fixes angular#12374.
@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent has pr labels Jul 28, 2018
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 28, 2018
…nside component with animations

Fixes the chip list losing its focus position if a chip is deleted while it's inside a component with animations.

Fixes angular#12374.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 28, 2018
…nside component with animations

Fixes the chip list losing its focus position if a chip is deleted while it's inside a component with animations.

Fixes angular#12374.
devversion pushed a commit to devversion/material2 that referenced this issue Aug 22, 2018
…nside component with animations

Fixes the chip list losing its focus position if a chip is deleted while it's inside a component with animations.

Fixes angular#12374.
jelbourn pushed a commit that referenced this issue Aug 23, 2018
…nside component with animations

Fixes the chip list losing its focus position if a chip is deleted while it's inside a component with animations.

Fixes #12374.
jelbourn pushed a commit that referenced this issue Aug 23, 2018
…nside component with animations (#12416)

Fixes the chip list losing its focus position if a chip is deleted while it's inside a component with animations.

Fixes #12374.
jelbourn pushed a commit that referenced this issue Aug 29, 2018
…nside component with animations (#12416)

Fixes the chip list losing its focus position if a chip is deleted while it's inside a component with animations.

Fixes #12374.
@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 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
1 participant