Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

fix(chips): editable chip gets removed after editing #11323

Merged
merged 1 commit into from
Jun 22, 2018
Merged

Conversation

Splaktar
Copy link
Member

@Splaktar Splaktar commented Jun 11, 2018

PR Checklist

Please check that your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added or this is not a bug fix / enhancement
  • Docs have been added, updated, or were not required

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Enhancement
[ ] Documentation content changes
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Editing chips is completely broken.

Issue Number:
Fixes #11298. Fixes #10392. Fixes #10532. Fixes #10664. Fixes #10879.

What is the new behavior?

  • use jQuery's contents() instead of children() since the chip content elements are text nodes and children() doesn't find text nodes.
  • improve double click detection when editable chips are enabled via jQuery/jQLite's dblclick Event
  • add/improve JSDoc and types
  • rename MdChipCtrl.isEditting to MdChipCtrl.isEditing

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@Splaktar Splaktar added type: bug a11y This issue is related to accessibility severity: regression This issue is related to a regression P2: required Issues that must be fixed. labels Jun 11, 2018
@Splaktar Splaktar added this to the 1.1.10 milestone Jun 11, 2018
@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Jun 11, 2018
@Splaktar Splaktar added the pr: merge ready This PR is ready for a caretaker to review label Jun 11, 2018
use jQuery's `contents()` instead of `children()`
since the chip content elements are text nodes
and `children()` doesn't find text nodes
improve double click detection when editable chips are enabled
add/improve JSDoc and types
rename `MdChipCtrl.isEditting` to `MdChipCtrl.isEditing`

Fixes #11298. Fixes #10392. Fixes #10532. Fixes #10664. Fixes #10879.
@jelbourn jelbourn merged commit 9cc165f into master Jun 22, 2018
@Splaktar Splaktar deleted the fixEditChip branch June 22, 2018 21:03
Splaktar added a commit that referenced this pull request Jul 9, 2018
- don't override defaults for scope variables which aren't defined
  - properly mark scope variables as optional in chips directive
  - the default inputAriaLabel, deleteHint, etc should not be rendered
- deleteHint in chip content broke the ability to edit chips
  - move deleteHint out of chip content and only aria-label on md-chip
- handle edge case in editing when the user clears the content and
  then enters new content

Fixes #11322. Relates to #11323.
jelbourn pushed a commit that referenced this pull request Jul 11, 2018
- don't override defaults for scope variables which aren't defined
  - properly mark scope variables as optional in chips directive
  - the default inputAriaLabel, deleteHint, etc should not be rendered
- deleteHint in chip content broke the ability to edit chips
  - move deleteHint out of chip content and only aria-label on md-chip
- handle edge case in editing when the user clears the content and
  then enters new content

Fixes #11322. Relates to #11323.
Splaktar added a commit that referenced this pull request Jul 31, 2018
use jQuery's `contents()` instead of `children()`
since the chip content elements are text nodes
and `children()` doesn't find text nodes
improve double click detection when editable chips are enabled
add/improve JSDoc and types
rename `MdChipCtrl.isEditting` to `MdChipCtrl.isEditing`

Fixes #11298. Fixes #10392. Fixes #10532. Fixes #10664. Fixes #10879.
Splaktar added a commit that referenced this pull request Jul 31, 2018
- don't override defaults for scope variables which aren't defined
  - properly mark scope variables as optional in chips directive
  - the default inputAriaLabel, deleteHint, etc should not be rendered
- deleteHint in chip content broke the ability to edit chips
  - move deleteHint out of chip content and only aria-label on md-chip
- handle edge case in editing when the user clears the content and
  then enters new content

Fixes #11322. Relates to #11323.
Splaktar added a commit that referenced this pull request Aug 2, 2018
- don't override defaults for scope variables which aren't defined
  - properly mark scope variables as optional in chips directive
  - the default inputAriaLabel, deleteHint, etc should not be rendered
- deleteHint in chip content broke the ability to edit chips
  - move deleteHint out of chip content and only aria-label on md-chip
- handle edge case in editing when the user clears the content and
  then enters new content

Fixes #11322. Relates to #11323.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a11y This issue is related to accessibility cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ P2: required Issues that must be fixed. pr: merge ready This PR is ready for a caretaker to review severity: regression This issue is related to a regression type: bug
Projects
None yet
3 participants