feat(chips): ability to disable chip addition (input). (closes #500) #547

JoshSchoen · 2017-05-03T18:51:09Z

Description

Feature Request: Turn off autocomplete for chips #500

What's included?

modified: src/platform/core/chips/chips.component.ts
modified: src/platform/core/chips/chips.component.html
modified: src/app/components/components/chips/chips.component.html
modified: src/app/components/components/chips/chips.component.ts

Test Steps

Checkout Branch
ng serve
Navigate to http://localhost:4200/#/components/chips
Click on Toggle Autocomplete Button
Make sure Autocomplete turns on and off

Manual Test

General Tests for Every PR

ng serve --aot still works.
npm run lint passes.
npm test passes and code coverage is not lower.
npm run build still works.

Screenshots or link to CodePen/Plunker/JSfiddle

… feature/chips

emoralesb05 · 2017-05-03T20:54:01Z

src/platform/core/chips/chips.component.ts

+   * autoComplete?: boolean
+   * Disables autocomplete. If it doesn't exist autocomplete defaults to true.
+   */
+  @Input('autoComplete') autoComplete: boolean = true;


So calling the input autocomplete can make it misleading about what it actually does since the autocomplete part is technically an add-on to the mdInput.

I think we should call the property allowAdd or something like this since later on we can possibly add later on an allowRemove which can enable/disable the x buttons from the chips.

Will do I will make the change to allowAdd and the one bellow.Thanks!

emoralesb05 · 2017-05-03T20:54:49Z

Also, can you fix the lint issues? 😄

JoshSchoen · 2017-05-03T22:32:11Z

@emoralesb05 updated autoComplete to allowAdd and fixed the lint issue. Does the dependency-ci check take a while or is that a manual thing?

emoralesb05 · 2017-05-03T23:46:10Z

Not sure why it hangs when its a PR from a fork 🤔 .. but dont worry about it haha 😄

Awesome dude! Gonna test the PR tonight 🎉

emoralesb05 · 2017-05-04T03:38:36Z

src/platform/core/chips/chips.component.html

@@ -9,6 +9,7 @@
        </md-icon>
      </md-basic-chip>
    </ng-template>
+    <div *ngIf="allowAdd">
    <md-input-container floatPlaceholder="never"


Can you add indentation for the content of the <div *ngIf="allowAdd">? 😄

emoralesb05 · 2017-05-04T03:40:38Z

There is a need to check for allowAdd in the focus() since it fails when focusing it when allowAdd is false.

e.g.

focus(): void {
    if (this.allowAdd) {
      this._inputChild.focus();
    }
}

emoralesb05 · 2017-05-04T03:45:22Z

src/platform/core/chips/chips.component.html

@@ -23,11 +24,13 @@
              (focus)="handleFocus()"
              (blur)="handleBlur()">
    </md-input-container>
+
    <md-autocomplete #autocomplete="mdAutocomplete">


There is a weird bug when you toggle the allowAdd to false and then back to true, once you focus the input the autocomplete doesnt appear until you type on it.

The fix is to put the md-autocomplete out of the <div *ngIf="allowAdd"> so its added as soon as the md-input is rendered without affecting the autocomplete rendering.

e. g.

<div *ngIf="allowAdd"> <md-input-container floatPlaceholder="never" [style.width.px]="readOnly ? 0 : null" [color]="matches ? 'primary' : 'warn'"> <input mdInput flex="100" #input [mdAutocomplete]="autocomplete" [formControl]="inputControl" [placeholder]="readOnly? '' : placeholder" (keydown)="_inputKeydown($event)" (keyup.enter)="addChip(input.value)" (focus)="handleFocus()" (blur)="handleBlur()"> </md-input-container> </div> <md-autocomplete #autocomplete="mdAutocomplete"> <ng-template let-item ngFor [ngForOf]="filteredItems | async"> <md-option (click)="addChip(input.value)" [value]="item">{{item}}</md-option> </ng-template> </md-autocomplete>

When I move the md-autocomplete outside <div *ngIf="allowAdd"> it breaks the ability add chips, the input.value is undefined. One way to get around this to visually with CSS hide it rather than removing it from the DOM with *ngIf. Let me know if this is acceptable otherwise I can look into an alternative.

The alternative approach would be to change (click)="addChip(input.value) to (click)="addChip(item). I might be wrong but for the click event wouldn't this be a little more concise to match the available list if we are only allowing the items in the list array anyway? Of course the input is a different thing but I don't see any errors and the focus event works as excepted when toggling from false back to true.

Yeah, I think it should use addChip(item) either way to be more consistent.

cool thanks!

JoshSchoen · 2017-05-04T03:55:12Z

Sounds good I'll make the changes and push again. Thanks!

…k to inputChild focus

kyleledbetter · 2017-05-04T22:57:04Z

small thing, but for the API should allowAdd be named something more like addChips? or since the real goal is disabling the input should it be more like disableInput?

kyleledbetter · 2017-05-04T22:58:28Z

Also, if the input is disabled, the .mat-input-underline should be hidden, since underline is to imply that there's an input text field

kyleledbetter · 2017-05-04T22:59:20Z

this would have an *ngIf conditional on it

covalent/src/platform/core/chips/chips.component.html

Line 32 in 1994564

<div class="mat-input-underline"

emoralesb05 · 2017-05-04T23:10:45Z

@kyleledbetter the reason i wanted to call it allowAdd is because after this there will be an addition of allowRemove. So its a cleaner API when you state what you are enabling/disabling in either case as an action, not as an internal component.

The ng1 chips called it md-removable, but they didnt have a sense of disabling the add since they used it as a mix of readOnly and md-removable i think

kyleledbetter · 2017-05-04T23:36:29Z

ok maybe it should be chipAddition and we can also add a flag to allow the removal of chips chipRemoval

JoshSchoen · 2017-05-05T00:20:01Z

Sure, I can make the changes. To be clear when you say chipRemoval does that mean when the user clicks on one of the chip list items when typing in autocomplete is the expected behavior to remove from the selected/filtered list rather than add to the selected/filtered list? If so does there need to be a toggle to add them back?

… feature/chips

JoshSchoen · 2017-05-05T13:09:22Z

@kyleledbetter @emoralesb05 I pushed the change to hide the .mat-input-underline and allowAdd to chipAddition I figured it's probably best to get that out of the way and do another PR for chipRemoval Let me know about the question in my last comment about expected chipRemoval behavior and I can take that on as well. Let me know if there is anything else. Thx!

emoralesb05 · 2017-05-05T16:52:45Z

@JoshSchoen ill create an issue later with the specifications of how chipRemoval should work.

Merging this for now. Nice work dude! 💥

Josh-Schoen and others added 9 commits May 3, 2017 12:17

added chips

9b1fbd0

added autocomplete boolean for chips component

1d19cde

added autocomplete boolean for chips component

4c5d6ba

added comment description

43bb340

added toogle autocomplete method for documentation and testing.

bcb3145

Merge branch 'develop' into feature/chips

f55d200

removed white spaces

fc1e306

removed blank lines

61d029e

Merge branch 'feature/chips' of github.com:JoshSchoen/covalent-1 into…

fd1d598

… feature/chips

emoralesb05 changed the title ~~Feature/chips~~ feat(chips): ability to disable chip addition (input). (closes #500) May 3, 2017

emoralesb05 suggested changes May 3, 2017

View reviewed changes

Josh-Schoen added 3 commits May 3, 2017 16:19

changed autoComplete to allowAdd to disable chips autocomplete

e5a3cf2

changed comment from autoComplte to allowAdd

f13979a

removed blank space

296dd60

emoralesb05 added this to the Beta 4 milestone May 3, 2017

emoralesb05 suggested changes May 4, 2017

View reviewed changes

changed autocomplete click event from inlut.value to item, added chec…

f9c8338

…k to inputChild focus

emoralesb05 approved these changes May 4, 2017

View reviewed changes

Merge branch 'develop' into feature/chips

b9e084e

Josh-Schoen added 3 commits May 5, 2017 07:54

added ngIf to .mat-input-underline

d661d1a

Merge branch 'feature/chips' of github.com:JoshSchoen/covalent-1 into…

4e1fe5a

… feature/chips

changed allowAdd to chipAddition

cfa7d62

emoralesb05 added the code lgtm label May 5, 2017

emoralesb05 merged commit 1c75d35 into Teradata:develop May 5, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(chips): ability to disable chip addition (input). (closes #500) #547

feat(chips): ability to disable chip addition (input). (closes #500) #547

JoshSchoen commented May 3, 2017 •

edited

Loading

emoralesb05 May 3, 2017

JoshSchoen May 3, 2017

emoralesb05 commented May 3, 2017 •

edited

Loading

JoshSchoen commented May 3, 2017

emoralesb05 commented May 3, 2017

emoralesb05 May 4, 2017 •

edited

Loading

emoralesb05 commented May 4, 2017

emoralesb05 May 4, 2017 •

edited

Loading

JoshSchoen May 4, 2017 •

edited

Loading

JoshSchoen May 4, 2017 •

edited

Loading

emoralesb05 May 4, 2017

JoshSchoen May 4, 2017

JoshSchoen commented May 4, 2017

kyleledbetter commented May 4, 2017

kyleledbetter commented May 4, 2017

kyleledbetter commented May 4, 2017

emoralesb05 commented May 4, 2017 •

edited

Loading

kyleledbetter commented May 4, 2017

JoshSchoen commented May 5, 2017

JoshSchoen commented May 5, 2017

emoralesb05 commented May 5, 2017

feat(chips): ability to disable chip addition (input). (closes #500) #547

feat(chips): ability to disable chip addition (input). (closes #500) #547

Conversation

JoshSchoen commented May 3, 2017 • edited Loading

Description

What's included?

Test Steps

Manual Test

General Tests for Every PR

Screenshots or link to CodePen/Plunker/JSfiddle

emoralesb05 May 3, 2017

Choose a reason for hiding this comment

JoshSchoen May 3, 2017

Choose a reason for hiding this comment

emoralesb05 commented May 3, 2017 • edited Loading

JoshSchoen commented May 3, 2017

emoralesb05 commented May 3, 2017

emoralesb05 May 4, 2017 • edited Loading

Choose a reason for hiding this comment

emoralesb05 commented May 4, 2017

emoralesb05 May 4, 2017 • edited Loading

Choose a reason for hiding this comment

JoshSchoen May 4, 2017 • edited Loading

Choose a reason for hiding this comment

JoshSchoen May 4, 2017 • edited Loading

Choose a reason for hiding this comment

emoralesb05 May 4, 2017

Choose a reason for hiding this comment

JoshSchoen May 4, 2017

Choose a reason for hiding this comment

JoshSchoen commented May 4, 2017

kyleledbetter commented May 4, 2017

kyleledbetter commented May 4, 2017

kyleledbetter commented May 4, 2017

emoralesb05 commented May 4, 2017 • edited Loading

kyleledbetter commented May 4, 2017

JoshSchoen commented May 5, 2017

JoshSchoen commented May 5, 2017

emoralesb05 commented May 5, 2017

JoshSchoen commented May 3, 2017 •

edited

Loading

emoralesb05 commented May 3, 2017 •

edited

Loading

emoralesb05 May 4, 2017 •

edited

Loading

emoralesb05 May 4, 2017 •

edited

Loading

JoshSchoen May 4, 2017 •

edited

Loading

JoshSchoen May 4, 2017 •

edited

Loading

emoralesb05 commented May 4, 2017 •

edited

Loading