From a8be5291bb9b460530d143d8606c5dfa02e75343 Mon Sep 17 00:00:00 2001 From: Manu MA Date: Fri, 2 Nov 2018 22:15:28 +0100 Subject: [PATCH] fix(label): placeholder + floating label (#16111) * fix(label): placeholder + floating label * fix placeholder type * update docs * uodate docs --- core/src/components.d.ts | 8 +- core/src/components/datetime/datetime.tsx | 1 + core/src/components/input/input.tsx | 5 +- core/src/components/input/readme.md | 2 +- core/src/components/label/label.scss | 1 + .../select-option/select-option.tsx | 2 +- core/src/components/select/select.tsx | 256 ++++++++---------- core/src/components/select/test/label/e2e.ts | 10 + .../components/select/test/label/index.html | 68 +++++ core/src/components/textarea/readme.md | 2 +- core/src/components/textarea/textarea.tsx | 5 +- core/src/utils/input-interface.ts | 1 - 12 files changed, 205 insertions(+), 156 deletions(-) create mode 100644 core/src/components/select/test/label/e2e.ts create mode 100644 core/src/components/select/test/label/index.html diff --git a/core/src/components.d.ts b/core/src/components.d.ts index e77e31b0e6..84c254da11 100644 --- a/core/src/components.d.ts +++ b/core/src/components.d.ts @@ -1766,7 +1766,7 @@ export namespace Components { /** * Instructional text that shows before the input has a value. */ - 'placeholder'?: string; + 'placeholder'?: string | null; /** * If `true`, the user cannot modify the value. */ @@ -1912,7 +1912,7 @@ export namespace Components { /** * Instructional text that shows before the input has a value. */ - 'placeholder'?: string; + 'placeholder'?: string | null; /** * If `true`, the user cannot modify the value. */ @@ -4670,7 +4670,7 @@ export namespace Components { /** * Instructional text that shows before the input has a value. */ - 'placeholder'?: string; + 'placeholder'?: string | null; /** * If `true`, the user cannot modify the value. */ @@ -4768,7 +4768,7 @@ export namespace Components { /** * Instructional text that shows before the input has a value. */ - 'placeholder'?: string; + 'placeholder'?: string | null; /** * If `true`, the user cannot modify the value. */ diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index b63baeb959..7f0a2d8240 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -247,6 +247,7 @@ export class Datetime implements ComponentInterface { this.ionStyle.emit({ 'interactive': true, 'datetime': true, + 'has-placeholder': this.placeholder != null, 'has-value': this.hasValue(), 'interactive-disabled': this.disabled, }); diff --git a/core/src/components/input/input.tsx b/core/src/components/input/input.tsx index bb8ff38d92..3d40f3271e 100644 --- a/core/src/components/input/input.tsx +++ b/core/src/components/input/input.tsx @@ -133,7 +133,7 @@ export class Input implements ComponentInterface { /** * Instructional text that shows before the input has a value. */ - @Prop() placeholder?: string; + @Prop() placeholder?: string | null; /** * If `true`, the user cannot modify the value. @@ -263,6 +263,7 @@ export class Input implements ComponentInterface { this.ionStyle.emit({ 'interactive': true, 'input': true, + 'has-placeholder': this.placeholder != null, 'has-value': this.hasValue(), 'has-focus': this.hasFocus, 'interactive-disabled': this.disabled, @@ -355,7 +356,7 @@ export class Input implements ComponentInterface { multiple={this.multiple} name={this.name} pattern={this.pattern} - placeholder={this.placeholder} + placeholder={this.placeholder || ''} results={this.results} readOnly={this.readonly} required={this.required} diff --git a/core/src/components/input/readme.md b/core/src/components/input/readme.md index ae91419743..f811703ae4 100644 --- a/core/src/components/input/readme.md +++ b/core/src/components/input/readme.md @@ -31,7 +31,7 @@ It is meant for text `type` inputs only, such as `"text"`, `"password"`, `"email | `multiple` | `multiple` | If `true`, the user can enter more than one value. This attribute applies when the type attribute is set to `"email"` or `"file"`, otherwise it is ignored. | `boolean \| undefined` | `undefined` | | `name` | `name` | The name of the control, which is submitted with the form data. | `string` | `this.inputId` | | `pattern` | `pattern` | A regular expression that the value is checked against. The pattern must match the entire value, not just some subset. Use the title attribute to describe the pattern to help the user. This attribute applies when the value of the type attribute is `"text"`, `"search"`, `"tel"`, `"url"`, `"email"`, or `"password"`, otherwise it is ignored. | `string \| undefined` | `undefined` | -| `placeholder` | `placeholder` | Instructional text that shows before the input has a value. | `string \| undefined` | `undefined` | +| `placeholder` | `placeholder` | Instructional text that shows before the input has a value. | `null \| string \| undefined` | `undefined` | | `readonly` | `readonly` | If `true`, the user cannot modify the value. | `boolean` | `false` | | `required` | `required` | If `true`, the user must fill in a value before submitting a form. | `boolean` | `false` | | `results` | `results` | This is a nonstandard attribute supported by Safari that only applies when the type is `"search"`. Its value should be a nonnegative decimal integer. | `number \| undefined` | `undefined` | diff --git a/core/src/components/label/label.scss b/core/src/components/label/label.scss index 4858adf261..86606fbd6a 100644 --- a/core/src/components/label/label.scss +++ b/core/src/components/label/label.scss @@ -76,6 +76,7 @@ } :host-context(.item-has-focus).label-floating, +:host-context(.item-has-placeholder).label-floating, :host-context(.item-has-value).label-floating { @include transform(translate3d(0, 0, 0), scale(.8)); } diff --git a/core/src/components/select-option/select-option.tsx b/core/src/components/select-option/select-option.tsx index f01d8f34fc..30b488dc02 100644 --- a/core/src/components/select-option/select-option.tsx +++ b/core/src/components/select-option/select-option.tsx @@ -37,7 +37,7 @@ export class SelectOption implements ComponentInterface { @Event() ionSelectOptionDidUnload!: EventEmitter; componentWillLoad() { - if (this.value == null) { + if (this.value === undefined) { this.value = this.el.textContent || ''; } } diff --git a/core/src/components/select/select.tsx b/core/src/components/select/select.tsx index 44d9cd58f1..0ebec9dd40 100644 --- a/core/src/components/select/select.tsx +++ b/core/src/components/select/select.tsx @@ -18,6 +18,7 @@ export class Select implements ComponentInterface { private inputId = `ion-sel-${selectIds++}`; private labelId?: string; private overlay?: OverlaySelect; + private didInit = false; @Element() el!: HTMLIonSelectElement; @@ -27,7 +28,6 @@ export class Select implements ComponentInterface { @State() isExpanded = false; @State() keyFocus = false; - @State() text = ''; /** * The mode determines which platform styles to use. @@ -120,140 +120,49 @@ export class Select implements ComponentInterface { @Watch('value') valueChanged() { - // this select value just changed - // double check the select option with this value is checked - if (this.value === undefined) { - // set to undefined - // ensure all that are checked become unchecked - this.childOpts.filter(o => o.selected).forEach(selectOption => { - selectOption.selected = false; + if (this.didInit) { + this.updateOptions(); + this.ionChange.emit({ + value: this.value, }); - this.text = ''; - - } else { - let hasChecked = false; - const texts: string[] = []; - - this.childOpts.forEach(selectOption => { - if ((Array.isArray(this.value) && this.value.includes(selectOption.value)) || (selectOption.value === this.value)) { - if (!selectOption.selected && (this.multiple || !hasChecked)) { - // correct value for this select option - // but this select option isn't checked yet - // and we haven't found a checked yet - // so CHECK IT! - selectOption.selected = true; - - } else if (!this.multiple && hasChecked && selectOption.selected) { - // somehow we've got multiple select options - // with the same value, but only one can be checked - selectOption.selected = false; - } - - // remember we've got a checked select option button now - hasChecked = true; - - } else if (selectOption.selected) { - // this select option doesn't have the correct value - // and it's also checked, so let's uncheck it - selectOption.selected = false; - } - - if (selectOption.selected) { - texts.push(selectOption.textContent || ''); - } - }); - - this.text = texts.join(', '); + this.emitStyle(); } - - // emit the new value - this.ionChange.emit({ - value: this.value, - text: this.text - }); - this.emitStyle(); } @Listen('ionSelectOptionDidLoad') - optLoad(ev: CustomEvent) { - const selectOption = ev.target as HTMLIonSelectOptionElement; - this.childOpts = Array.from(this.el.querySelectorAll('ion-select-option')); - - if (this.value != null && (Array.isArray(this.value) && this.value.includes(selectOption.value)) || (selectOption.value === this.value)) { - // this select has a value and this - // option equals the correct select value - // so let's check this select option - selectOption.selected = true; - - } else if (Array.isArray(this.value) && this.multiple && selectOption.selected) { - // if the value is an array we need to push the option on - this.value.push(selectOption.value); - - } else if (this.value === undefined && selectOption.selected) { - // this select does not have a value - // but this select option is checked, so let's set the - // select's value from the checked select option - this.value = selectOption.value; - - } else if (selectOption.selected) { - // if it doesn't match one of the above cases, but the - // select option is still checked, then we need to uncheck it - selectOption.selected = false; - } - } - @Listen('ionSelectOptionDidUnload') - optUnload(ev: CustomEvent) { - const index = this.childOpts.indexOf(ev.target as HTMLIonSelectOptionElement); - if (index > -1) { - this.childOpts.splice(index, 1); + async selectOptionChanged() { + await this.loadOptions(); + if (this.didInit) { + this.updateOptions(); } } - @Listen('ionSelect') - onSelect(ev: CustomEvent) { - // ionSelect only come from the checked select option - this.childOpts.forEach(selectOption => { - if (selectOption === ev.target) { - this.value = selectOption.value; - } else { - selectOption.selected = false; - } - }); - } + async componentDidLoad() { + await this.loadOptions(); - componentWillLoad() { - if (!this.value) { - this.value = this.multiple ? [] : undefined; - } - } - componentDidLoad() { const label = this.getLabel(); if (label) { this.labelId = label.id = this.name + '-lbl'; } - if (this.multiple) { - // there are no values set at this point - // so check to see who should be selected - const checked = this.childOpts.filter(o => o.selected); - - (this.value as string[]).length = 0; - checked.forEach(o => { - // doing this instead of map() so we don't - // fire off an unnecessary change event - (this.value as string[]).push(o.value); - }); - this.text = checked.map(o => o.textContent).join(', '); - - } else { - const checked = this.childOpts.find(o => o.selected); - if (checked) { - this.value = checked.value; - this.text = checked.textContent || ''; + if (this.value === undefined) { + if (this.multiple) { + // there are no values set at this point + // so check to see who should be selected + const checked = this.childOpts.filter(o => o.selected); + this.value = checked.map(o => o.value); + } else { + const checked = this.childOpts.find(o => o.selected); + if (checked) { + this.value = checked.value; + } } } + this.updateOptions(); this.emitStyle(); + this.el.forceUpdate(); + this.didInit = true; } /** @@ -285,14 +194,6 @@ export class Select implements ComponentInterface { return this.openAlert(); } - private getLabel() { - const item = this.el.closest('ion-item'); - if (item) { - return item.querySelector('ion-label'); - } - return null; - } - private async openPopover(ev: UIEvent) { const interfaceOptions = this.interfaceOptions; @@ -339,6 +240,7 @@ export class Select implements ComponentInterface { } as ActionSheetButton; }); + // Add "cancel" button actionSheetButtons.push({ text: this.cancelText, role: 'cancel', @@ -425,6 +327,58 @@ export class Select implements ComponentInterface { return overlay.dismiss(); } + private async loadOptions() { + this.childOpts = await Promise.all( + Array.from(this.el.querySelectorAll('ion-select-option')).map(o => o.componentOnReady()) + ); + } + + private updateOptions() { + // iterate all options, updating the selected prop + let canSelect = true; + for (const selectOption of this.childOpts) { + const selected = canSelect && isOptionSelected(this.value, selectOption.value); + selectOption.selected = selected; + + // if current option is selected and select is single-option, we can't select + // any option more + if (selected && !this.multiple) { + canSelect = false; + } + } + } + + private getLabel() { + const item = this.el.closest('ion-item'); + if (item) { + return item.querySelector('ion-label'); + } + return null; + } + + private hasValue(): boolean { + return this.getText() !== ''; + } + + private getText(): string { + const selectedText = this.selectedText; + if (selectedText != null && selectedText !== '') { + return selectedText; + } + return generateText(this.childOpts, this.value); + } + + private emitStyle() { + this.ionStyle.emit({ + 'interactive': true, + 'select': true, + 'has-placeholder': this.placeholder != null, + 'has-value': this.hasValue(), + 'interactive-disabled': this.disabled, + 'select-disabled': this.disabled + }); + } + private onKeyUp = () => { this.keyFocus = true; } @@ -438,23 +392,6 @@ export class Select implements ComponentInterface { this.ionBlur.emit(); } - hasValue(): boolean { - if (Array.isArray(this.value)) { - return this.value.length > 0; - } - return (this.value != null && this.value !== undefined && this.value !== ''); - } - - private emitStyle() { - this.ionStyle.emit({ - 'interactive': true, - 'select': true, - 'has-value': this.hasValue(), - 'interactive-disabled': this.disabled, - 'select-disabled': this.disabled - }); - } - hostData() { return { class: { @@ -469,8 +406,7 @@ export class Select implements ComponentInterface { renderHiddenInput(this.el, this.name, parseValue(this.value), this.disabled); let addPlaceholderClass = false; - - let selectText = this.selectedText || this.text; + let selectText = this.getText(); if (selectText === '' && this.placeholder != null) { selectText = this.placeholder; addPlaceholderClass = true; @@ -522,4 +458,36 @@ function parseValue(value: any) { return value.toString(); } +function isOptionSelected(currentValue: any[] | any, optionValue: any) { + if (currentValue === undefined) { + return false; + } + if (Array.isArray(currentValue)) { + return currentValue.includes(optionValue); + } else { + return currentValue === optionValue; + } +} + +function generateText(opts: HTMLIonSelectOptionElement[], value: any | any[]) { + if (value === undefined) { + return ''; + } + if (Array.isArray(value)) { + return value + .map(v => textForValue(opts, v)) + .filter(opt => opt !== null) + .join(', '); + } else { + return textForValue(opts, value) || ''; + } +} + +function textForValue(opts: HTMLIonSelectOptionElement[], value: any): string | null { + const selectOpt = opts.find(opt => opt.value === value); + return selectOpt + ? selectOpt.textContent + : null; +} + let selectIds = 0; diff --git a/core/src/components/select/test/label/e2e.ts b/core/src/components/select/test/label/e2e.ts new file mode 100644 index 0000000000..e3290d9312 --- /dev/null +++ b/core/src/components/select/test/label/e2e.ts @@ -0,0 +1,10 @@ +import { newE2EPage } from '@stencil/core/testing'; + +it('select: label', async () => { + const page = await newE2EPage({ + url: '/src/components/select/test/label?ionic:_testing=true' + }); + + const compare = await page.compareScreenshot(); + expect(compare).toMatchScreenshot(); +}); diff --git a/core/src/components/select/test/label/index.html b/core/src/components/select/test/label/index.html new file mode 100644 index 0000000000..67a5389f3a --- /dev/null +++ b/core/src/components/select/test/label/index.html @@ -0,0 +1,68 @@ + + + + + + Select - Basic + + + + + + + + + + + + Select - Labels + + + + + + + Gender + + Female + Male + + + + + Hair Color + + Brown + Empty Value + Black + Red + + + + + Hair Color + + Brown + Blonde + Black + Red + + + + + Hair Color + + Brown + Blonde + Black + Red + + + + + + + + + + diff --git a/core/src/components/textarea/readme.md b/core/src/components/textarea/readme.md index daf1c27108..9c62455779 100644 --- a/core/src/components/textarea/readme.md +++ b/core/src/components/textarea/readme.md @@ -26,7 +26,7 @@ The textarea component accepts the [native textarea attributes](https://develope | `minlength` | `minlength` | If the value of the type attribute is `text`, `email`, `search`, `password`, `tel`, or `url`, this attribute specifies the minimum number of characters that the user can enter. | `number \| undefined` | `undefined` | | `mode` | `mode` | The mode determines which platform styles to use. | `"ios" \| "md"` | `undefined` | | `name` | `name` | The name of the control, which is submitted with the form data. | `string` | `this.inputId` | -| `placeholder` | `placeholder` | Instructional text that shows before the input has a value. | `string \| undefined` | `undefined` | +| `placeholder` | `placeholder` | Instructional text that shows before the input has a value. | `null \| string \| undefined` | `undefined` | | `readonly` | `readonly` | If `true`, the user cannot modify the value. | `boolean` | `false` | | `required` | `required` | If `true`, the user must fill in a value before submitting a form. | `boolean` | `false` | | `rows` | `rows` | The number of visible text lines for the control. | `number \| undefined` | `undefined` | diff --git a/core/src/components/textarea/textarea.tsx b/core/src/components/textarea/textarea.tsx index 2e5f1bba50..f3c22280f8 100644 --- a/core/src/components/textarea/textarea.tsx +++ b/core/src/components/textarea/textarea.tsx @@ -87,7 +87,7 @@ export class Textarea implements ComponentInterface { /** * Instructional text that shows before the input has a value. */ - @Prop() placeholder?: string; + @Prop() placeholder?: string | null; /** * If `true`, the user cannot modify the value. @@ -187,6 +187,7 @@ export class Textarea implements ComponentInterface { 'textarea': true, 'input': true, 'interactive-disabled': this.disabled, + 'has-placeholder': this.placeholder != null, 'has-value': this.hasValue(), 'has-focus': this.hasFocus }); @@ -272,7 +273,7 @@ export class Textarea implements ComponentInterface { maxLength={this.maxlength} minLength={this.minlength} name={this.name} - placeholder={this.placeholder} + placeholder={this.placeholder || ''} readOnly={this.readonly} required={this.required} spellCheck={this.spellcheck} diff --git a/core/src/utils/input-interface.ts b/core/src/utils/input-interface.ts index f6a49f6b97..a88419b548 100644 --- a/core/src/utils/input-interface.ts +++ b/core/src/utils/input-interface.ts @@ -12,7 +12,6 @@ export interface InputChangeEvent { export interface SelectInputChangeEvent { value: any | any[] | undefined | null; - text: string | undefined | null; } export interface StyleEvent {