refactor(mdc-chips): Base isEmptyInput check on chipInput value. #16977
Conversation
Ensures that grids using custom input elements don't break on _isEmptyInput checks. (empty() in chip-input.ts uses the same !input.value logic, so there's no change for existing implementations.)
googlebot
added
the
cla: yes
| if (element && element.nodeName.toLowerCase() === 'input') { | ||
| let input = element as HTMLInputElement; | ||
| return !input.value; | ||
| if (element && element.id === this._chipInput!.id) { |
There was a problem hiding this comment.
I'm not sure why we need this check in the first place. Wouldn't the chip grid always be associated with the proper input anyway because of its DOM structure?
There was a problem hiding this comment.
This method is called in a generic _keyDown handler, so we're attempting to determine first if the element that received the keyDown event is the input element, and second, whether that input is empty.
There was a problem hiding this comment.
Makes sense, but it seems a little fragile because it assumes that the input will have an ID. We could check it by identity instead (element === this._chipInput).
There was a problem hiding this comment.
The problem there comes that _chipInput is not actually a reference to an html element, it's an interface defined in chip-text-control.ts that's provided to the grid (that provides the necessary references/methods to drive interaction with the input), so the two aren't comparable. For context, _chipInput must always have an id, it's a required field. Inputs that use a matChipInputFor directive have the id generated and assigned automatically. It would be the onus of any person using a custom input to properly implement the interface and assign the id.
crisbeto
added
P3
|
I see that you just added the If you want your PR to be merged, it has to pass all the CI checks. If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help. |
|
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. |
Ensures that grids using custom input elements don't break on _isEmptyInput checks. (empty() in chip-input.ts uses the same !input.value logic, so there's no change for existing implementations.)