prototype(chips): MDC chip APIs match existing APIs #16145

vanessanschmitt · 2019-05-29T04:49:59Z

Work in progress, just looking for feedback! Will keep updating as I work.

src/material-experimental/mdc-chips/chip-cell.ts

vanessanschmitt · 2019-06-14T18:43:11Z

Hey! I pushed the new MDC chip classes with the structure we discussed. I'm looking to get feedback on this partial implementation, and ideally get it submitted on its own.

Known big missing things:

MatChipGrid/MatChipRow do not have focus management/arrow key navigation implemented yet. I'm thinking I'll have to write some custom logic for them, since I don't see a grid equivalent of FocusKeyManager. Wanted to check in with you all before I go ahead and write that!

Open questions:

I currently have MatChipListbox implementing just ControlValueAccessor, and MatChipGrid implementing both ControlValueAccessor and MatFormFieldControl. Does that make sense?

Next steps:

Add keyboard navigation/focus management for MatChipGrid/MatChipRow (separate PR)
Copy existing tests and ensure they all pass (separate PR)

jelbourn

Overall looks good to me. This made me realize we have some stale patterns in the chip-list that we should clean up eventually (like overuse of any in the value space), but that's a job for another time.

src/material-experimental/mdc-chips/_mdc-chips.scss

jelbourn · 2019-06-14T22:34:31Z

src/material-experimental/mdc-chips/chip-default-options.ts

+/** Default options, for the chips module, that can be overridden. */
+export interface MatChipsDefaultOptions {
+  /** The list of key codes that will trigger a chipEnd event. */
+  separatorKeyCodes: number[] | Set<number>;


FYI we decided just two days ago that we're going to change the way we handle these separator keys in the chips. It turns out that virtual keyboards on iOS and Android don't actually send keyCodes (or Keyboardevent.key), so we're going to deprecate this API and and change to something like delimiterCharacters and instead base the behavior off of the value in the input, with special casing for tab and enter. No action required for right now, but worth keeping in mind when you get to the board handling. I'll be sure to keep you in the loop when we figure out the final behavior we want.

cc @josephperrott

src/material-experimental/mdc-chips/chip-grid.ts

src/material-experimental/mdc-chips/chip-listbox.ts

src/material-experimental/mdc-chips/chip-option.html

src/material-experimental/mdc-chips/chip-row.ts

src/material-experimental/mdc-chips/chip.ts

jelbourn · 2019-06-14T23:20:57Z

src/material-experimental/mdc-chips/chip.ts

+    notifyTrailingIconInteraction: () => this.trailingIconInteraction.emit(this.id),
+    notifyRemoval: () => this.removed.emit({chip: this}),
+    getComputedStyleValue: (propertyName) => {
+      return window.getComputedStyle(this._mdcChip.nativeElement).getPropertyValue(propertyName);


@mmalerba will this cause a problem with SSR?

src/material-experimental/mdc-chips/chip-grid.ts

jelbourn

Last a few last things I noticed on another pass

src/material-experimental/mdc-chips/chips.scss

src/material-experimental/mdc-chips/chip.ts

src/material-experimental/mdc-chips/chip-option.ts

src/material-experimental/mdc-chips/chip-row.ts

jelbourn · 2019-06-18T18:44:32Z

For the build issue:

It's missing a dep in the sass_binary: "//src/material-experimental/mdc-helpers:mdc_helpers_scss_lib"
Also, the version of material-components-web in the package.json needs to be updated. It looks like ^2.3.1 will work

vanessanschmitt · 2019-06-18T22:09:27Z

I updated the version of material-components-web in package.json, and 'yarn test' started passing locally. When I pushed to Github, I got yarn.lock out of date errors. I ran 'yarn install' to update it, and pushed again. Now everything on circle cl is failing at the restore cache step with permission denied errors. Is this expected?

jelbourn · 2019-06-18T23:24:46Z

Looks like the CI might just be a transient error? I'd try rebasing and pushing again

cc @devversion

devversion · 2019-06-19T08:17:11Z

@jelbourn I think that I caused this issue while working on the Angular update where I might have stored a new cache entry in CircleCI (with a new docker image user). Rebasing should ensure that a more recent cache entry is used (which should not be polluted by my Angular update PR)

vanessanschmitt · 2019-06-19T16:44:46Z

I rebased and pushed again, but I'm still getting the same permission denied errors.

commit 273f8a2ecf6dde4b5db78e27ced45d093126f4fb (HEAD -> devel, origin/devel)
Author: Vanessa Schmitt <[email protected]>
Date:   Fri Jun 14 11:16:45 2019 -0700

    prototype(chips): MDC chip APIs match existing APIs

commit 7a04788cd32e1b37693e4dd9784ca986c5d28f85 (upstream/master, master)
Author: Kristiyan Kostadinov <[email protected]>
Date:   Wed Jun 19 07:25:07 2019 +0200

cc @devversion

jelbourn · 2019-06-19T21:40:26Z

@devversion any further ideas? I tried this on another PR (#16335) and the same thing happens. I don't see anything in the CircleCI UI to manually clear the cache like existed in Travis. Could it have anything to do with the yarn version?

devversion · 2019-06-23T21:02:12Z

@vanessanschmitt The issues have been sorted out. Can you please try to rebase? Thank you!

jelbourn

LGTM

vanessanschmitt · 2019-06-25T21:46:10Z

related to #16173

angular-automatic-lock-bot · 2019-09-11T01:00:58Z

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.}

vanessanschmitt requested a review from mmalerba as a code owner May 29, 2019 04:49

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 29, 2019

vanessanschmitt force-pushed the devel branch from 4fe86d1 to 8a1dff2 Compare May 29, 2019 17:49

jelbourn reviewed May 30, 2019

View reviewed changes

src/material-experimental/mdc-chips/chip-cell.ts Outdated Show resolved Hide resolved

src/material-experimental/mdc-chips/chip-cell.ts Outdated Show resolved Hide resolved

src/material-experimental/mdc-chips/chip-cell.ts Outdated Show resolved Hide resolved

vanessanschmitt force-pushed the devel branch 2 times, most recently from 14f0237 to bf5e72f Compare June 4, 2019 16:04

vanessanschmitt force-pushed the devel branch from bf5e72f to 2cc934d Compare June 14, 2019 18:20

vanessanschmitt changed the title ~~prototype(mat-chip-list): WIP MDC chips~~ prototype(chips): MDC chip APIs match existing APIs Jun 14, 2019

vanessanschmitt force-pushed the devel branch 3 times, most recently from bce13bd to e80c090 Compare June 14, 2019 22:48

jelbourn reviewed Jun 14, 2019

View reviewed changes

jelbourn requested a review from crisbeto June 14, 2019 23:22

vanessanschmitt force-pushed the devel branch 2 times, most recently from 7d6145d to 687d6a7 Compare June 17, 2019 22:08

This comment has been minimized.

Sign in to view

mmalerba reviewed Jun 17, 2019

View reviewed changes

src/material-experimental/mdc-chips/chip-grid.ts Outdated Show resolved Hide resolved

src/material-experimental/mdc-chips/chip-grid.ts Outdated Show resolved Hide resolved

vanessanschmitt force-pushed the devel branch from 687d6a7 to 6861735 Compare June 18, 2019 16:59

jelbourn reviewed Jun 18, 2019

View reviewed changes

vanessanschmitt force-pushed the devel branch 3 times, most recently from 4ddcc2f to 4aa4c84 Compare June 18, 2019 22:04

vanessanschmitt force-pushed the devel branch from 4aa4c84 to 273f8a2 Compare June 19, 2019 16:37

vanessanschmitt force-pushed the devel branch from 273f8a2 to 64f7802 Compare June 19, 2019 17:50

prototype(chips): MDC chip APIs match existing APIs

867ba09

vanessanschmitt force-pushed the devel branch from 64f7802 to 867ba09 Compare June 24, 2019 16:49

jelbourn added the target: minor This PR is targeted for the next minor release label Jun 24, 2019

jelbourn approved these changes Jun 24, 2019

View reviewed changes

jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Jun 24, 2019

jelbourn merged commit 86aad59 into angular:master Jun 24, 2019

vanessanschmitt deleted the devel branch June 24, 2019 21:33

vanessanschmitt mentioned this pull request Jun 27, 2019

feature(material-experimental/chips) Add grid keyboard shortcuts #16384

Merged

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

prototype(chips): MDC chip APIs match existing APIs #16145

prototype(chips): MDC chip APIs match existing APIs #16145

vanessanschmitt commented May 29, 2019

vanessanschmitt commented Jun 14, 2019

jelbourn left a comment

jelbourn Jun 14, 2019

jelbourn Jun 14, 2019

This comment has been minimized.

jelbourn left a comment •

edited

Loading

jelbourn commented Jun 18, 2019

vanessanschmitt commented Jun 18, 2019

jelbourn commented Jun 18, 2019

devversion commented Jun 19, 2019

vanessanschmitt commented Jun 19, 2019 •

edited

Loading

jelbourn commented Jun 19, 2019

devversion commented Jun 23, 2019

jelbourn left a comment

vanessanschmitt commented Jun 25, 2019

angular-automatic-lock-bot bot commented Sep 11, 2019

prototype(chips): MDC chip APIs match existing APIs #16145

prototype(chips): MDC chip APIs match existing APIs #16145

Conversation

vanessanschmitt commented May 29, 2019

vanessanschmitt commented Jun 14, 2019

jelbourn left a comment

Choose a reason for hiding this comment

jelbourn Jun 14, 2019

Choose a reason for hiding this comment

jelbourn Jun 14, 2019

Choose a reason for hiding this comment

This comment has been minimized.

jelbourn left a comment • edited Loading

Choose a reason for hiding this comment

jelbourn commented Jun 18, 2019

vanessanschmitt commented Jun 18, 2019

jelbourn commented Jun 18, 2019

devversion commented Jun 19, 2019

vanessanschmitt commented Jun 19, 2019 • edited Loading

jelbourn commented Jun 19, 2019

devversion commented Jun 23, 2019

jelbourn left a comment

Choose a reason for hiding this comment

vanessanschmitt commented Jun 25, 2019

angular-automatic-lock-bot bot commented Sep 11, 2019

jelbourn left a comment •

edited

Loading

vanessanschmitt commented Jun 19, 2019 •

edited

Loading