Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 17 commits into from
May 5, 2017
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/app/components/components/chips/chips.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<ng-template md-tab-label>Demo</ng-template>
<div class="push">
<div class="md-body-1">Type and select a preset option:</div>
<td-chips [items]="items" [(ngModel)]="itemsRequireMatch" placeholder="Enter autocomplete strings" [readOnly]="readOnly" requireMatch></td-chips>
<td-chips [autoComplete]="autoComplete" [items]="items" [(ngModel)]="itemsRequireMatch" placeholder="Enter autocomplete strings" [readOnly]="readOnly" requireMatch></td-chips>
</div>
</md-tab>
<md-tab>
Expand All @@ -16,7 +16,7 @@
<p>HTML:</p>
<td-highlight lang="html">
<![CDATA[
<td-chips [items]="items" [(ngModel)]="itemsRequireMatch" placeholder="Enter autocomplete strings" [readOnly]="readOnly" requireMatch></td-chips>
<td-chips [items]="items" [autoComplete]="autoComplete" [(ngModel)]="itemsRequireMatch" placeholder="Enter autocomplete strings" [readOnly]="readOnly" requireMatch></td-chips>
]]>
</td-highlight>
<p>Typescript:</p>
Expand Down Expand Up @@ -53,6 +53,7 @@
<md-divider></md-divider>
<md-card-actions>
<button md-button color="primary" (click)="toggleReadOnly()" class="text-upper">Toggle ReadOnly</button>
<button md-button color="primary" (click)="toggleAutocomplete()" class="text-upper">Toggle Autocomplete</button>
</md-card-actions>
</md-card>
<md-card>
Expand Down
8 changes: 8 additions & 0 deletions src/app/components/components/chips/chips.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,14 @@ export class ChipsDemoComponent {
Sends chip value as event.`,
name: 'remove?',
type: 'function',
}, {
description: `Disables autocomplete. If it doesn't exist autocomplete defaults to true.`,
name: 'autocomplete?',
type: 'boolean',
}];

readOnly: boolean = false;
autoComplete: boolean = true;

items: string[] = [
'stepper',
Expand All @@ -63,5 +68,8 @@ export class ChipsDemoComponent {
toggleReadOnly(): void {
this.readOnly = !this.readOnly;
}
toggleAutocomplete(): void {
this.autoComplete = !this.autoComplete;
}

}
3 changes: 3 additions & 0 deletions src/platform/core/chips/chips.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
</md-icon>
</md-basic-chip>
</ng-template>
<div *ngIf="autoComplete">
<md-input-container floatPlaceholder="never"
Copy link
Contributor

@emoralesb05 emoralesb05 May 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

[style.width.px]="readOnly ? 0 : null"
[color]="matches ? 'primary' : 'warn'">
Expand All @@ -23,11 +24,13 @@
(focus)="handleFocus()"
(blur)="handleBlur()">
</md-input-container>

<md-autocomplete #autocomplete="mdAutocomplete">
Copy link
Contributor

@emoralesb05 emoralesb05 May 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Copy link
Contributor Author

@JoshSchoen JoshSchoen May 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@JoshSchoen JoshSchoen May 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool thanks!

<ng-template let-item ngFor [ngForOf]="filteredItems | async">
<md-option (click)="addChip(input.value)" [value]="item">{{item}}</md-option>
</ng-template>
</md-autocomplete>
</div>
</md-chip-list>
<div class="mat-input-underline"
[class.mat-disabled]="readOnly">
Expand Down
18 changes: 15 additions & 3 deletions src/platform/core/chips/chips.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export class TdChipsComponent implements ControlValueAccessor, DoCheck, OnInit {
* Enables Autocompletion with the provided list of strings.
*/
@Input('items') items: string[] = [];


/**
* requireMatch?: boolean
Expand All @@ -79,6 +80,8 @@ export class TdChipsComponent implements ControlValueAccessor, DoCheck, OnInit {
return this._requireMatch;
}



/**
* readOnly?: boolean
* Disables the chips input and chip removal icon.
Expand All @@ -96,6 +99,12 @@ export class TdChipsComponent implements ControlValueAccessor, DoCheck, OnInit {
return this._readOnly;
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


/**
* placeholder?: string
* Placeholder for the autocomplete input.
Expand Down Expand Up @@ -227,6 +236,7 @@ export class TdChipsComponent implements ControlValueAccessor, DoCheck, OnInit {
case DELETE:
case BACKSPACE:
/** Check to see if input is empty when pressing left arrow to move to the last chip */

if (!this._inputChild.value) {
this._focusLastChip();
event.preventDefault();
Expand Down Expand Up @@ -257,7 +267,7 @@ export class TdChipsComponent implements ControlValueAccessor, DoCheck, OnInit {
* Checks if deleting last single chip, to focus input afterwards
* Else check if its not the last chip of the list to focus the next one.
*/
if (index === (this._totalChips - 1) && index === 0) {
if (index === (this._totalChips - 1) && index === 0 && this.autoComplete) {
this.focus();
} else if (index < (this._totalChips - 1)) {
this._focusChip(index + 1);
Expand All @@ -267,20 +277,22 @@ export class TdChipsComponent implements ControlValueAccessor, DoCheck, OnInit {
break;
case LEFT_ARROW:
/** Check to see if left arrow was pressed while focusing the first chip to focus input next */
if (index === 0) {
if (index === 0 && this.autoComplete) {
this.focus();
event.stopPropagation();
}
break;
case RIGHT_ARROW:
/** Check to see if right arrow was pressed while focusing the last chip to focus input next */
if (index === (this._totalChips - 1)) {
if (index === (this._totalChips - 1) && this.autoComplete) {
this.focus();
event.stopPropagation();
}
break;
case ESCAPE:
if (this.autoComplete) {
this.focus();
}
break;
default:
// default
Expand Down