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

fix(chips): editing chips works again #11364

Merged
merged 1 commit into from
Jul 11, 2018
Merged

fix(chips): editing chips works again #11364

merged 1 commit into from
Jul 11, 2018

Conversation

Splaktar
Copy link
Contributor

@Splaktar Splaktar commented Jul 9, 2018 •
edited
Loading

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?

Pressing the arrow keys, clicking the mouse, or pressing the backspace key in edit mode would cause the chips to become unresponsive or leave an empty chip behind.

Issue Number:
Fixes #11322. Relates to #11323.

What is the new behavior?

  • Navigation through the chip content text with arrow keys and the mouse is now supported.
  • Clearing the chip content text and writing new text now works as expected.
  • Backspace key works as expected and there is no longer an invisible text element inserted into the chip contents (the visually hidden deleteHint).
  • 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

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

This fixes some of the regressions introduced as part of the a11y changes in 1.1.2.

I spent a good deal of time trying to add the first two tests for this edit chips feature, but I was unsuccessful. I was unable to fire the right events and find/focus the right elements to enter edit mode, change the text, and exit edit mode.

@Splaktar
fix(chips): editing chips works again
96fc81b 
- 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.
@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Jul 9, 2018
@Splaktar Splaktar added this to the 1.1.11 milestone Jul 9, 2018
@Splaktar Splaktar added severity: regression This issue is related to a regression P2: required Issues that must be fixed. labels Jul 9, 2018
@Splaktar Splaktar mentioned this pull request Jul 9, 2018
Closed
@Splaktar Splaktar added type: bug pr: merge ready This PR is ready for a caretaker to review labels Jul 9, 2018
@jelbourn jelbourn merged commit 97455f5 into master Jul 11, 2018
@Splaktar Splaktar deleted the fixEditChips branch July 11, 2018 23:32
Splaktar added a commit that referenced this pull request Jul 31, 2018
@Splaktar
fix(chips): editing chips works again (#11364)
2b6c372 
- 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
@Splaktar
fix(chips): editing chips works again (#11364)
9f38c9a 
- 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.
Reviewers
No reviews
Assignees

@jelbourn jelbourn

Labels
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
Milestone
1.1.11
Development

Successfully merging this pull request may close these issues.

chips(editable): arrow keys and mouse clicks break edit mode
3 participants
@Splaktar @jelbourn @googlebot