diff --git a/changelog/unreleased/enhancement-octextinput-props b/changelog/unreleased/enhancement-octextinput-props new file mode 100644 index 000000000..4ca814e94 --- /dev/null +++ b/changelog/unreleased/enhancement-octextinput-props @@ -0,0 +1,10 @@ +Enhancement: Streamline OcTextInput + +We have updated the OcTextInput component to be in line with other form components in the design system. + +- Fixed the clear button being visible even when element is disabled +- Made clear button emit null instead of empty string +- Made input/change event handling a bit more consistent +- Added defaultValue prop that is for now passed as placeholder to the input field + +https://github.com/owncloud/owncloud-design-system/pull/1636 diff --git a/src/components/molecules/OcTextInput/OcTextInput.spec.js b/src/components/molecules/OcTextInput/OcTextInput.spec.js index 79a5fecfe..72808f3b8 100644 --- a/src/components/molecules/OcTextInput/OcTextInput.spec.js +++ b/src/components/molecules/OcTextInput/OcTextInput.spec.js @@ -165,16 +165,29 @@ describe("OcTextInput", () => { describe("clear input", () => { it("has no clear button when it is disabled", () => { - const wrapper = getShallowWrapper({ value: "non-empty-value" }) + const wrapper = getShallowWrapper({ + value: "non-empty-value", + clearButtonEnabled: true, + disabled: true, + }) expect(wrapper.find(selectors.clearInputButton).exists()).toBeFalsy() }) - it("has no clear button when it is enabled but the input is empty", () => { + it("has clear button when it is enabled but the input is an empty string but not null", () => { const wrapper = getShallowWrapper({ clearButtonEnabled: true, value: "", }) + expect(wrapper.find(selectors.clearInputButton).exists()).toBeTruthy() + }) + + it("has no clear button when it is enabled but the input is null", () => { + const wrapper = getShallowWrapper({ + clearButtonEnabled: true, + value: null, + }) + expect(wrapper.find(selectors.clearInputButton).exists()).toBeFalsy() }) @@ -203,8 +216,11 @@ describe("OcTextInput", () => { await btn.trigger("click") - expect(wrapper.emitted().input[0][0]).toEqual("") - expect(input.element.value).toEqual("") + // value as data is supposed to be `null` + expect(wrapper.emitted().input[0][0]).toEqual(null) + // value in DOM would be the empty string if two way binding was used + // by just passing in the value it should remain unchanged + expect(input.element.value).toEqual("non-empty-value") expect(document.activeElement.id).toBe(input.element.id) }) }) diff --git a/src/components/molecules/OcTextInput/OcTextInput.vue b/src/components/molecules/OcTextInput/OcTextInput.vue index 20ab0c2a6..74bd9abf6 100644 --- a/src/components/molecules/OcTextInput/OcTextInput.vue +++ b/src/components/molecules/OcTextInput/OcTextInput.vue @@ -13,8 +13,10 @@ 'oc-text-input-danger': !!errorMessage, }" :type="type" - :value="value" + :value="displayValue" + :disabled="disabled" v-on="listeners" + @change="onChange($event.target.value)" @input="onInput($event.target.value)" @focus="onFocus($event.target)" /> @@ -111,6 +113,23 @@ export default { required: false, default: "", }, + /** + * Value to show when no value is provided + * This does not set `value` automatically. + * The user needs to explicitly enter a text to set it as `value`. + */ + defaultValue: { + type: String, + required: false, + default: null, + }, + /** + * Disables the input field + */ + disabled: { + type: Boolean, + default: false, + }, /** * Accessible of the form input field, via aria-label. **/ @@ -169,6 +188,7 @@ export default { const listeners = this.$listeners // Delete listeners for events which are emitted via methods + delete listeners["change"] delete listeners["input"] delete listeners["focus"] @@ -182,6 +202,10 @@ export default { if (!!this.warningMessage || !!this.errorMessage || !!this.descriptionMessage) { additionalAttrs["aria-describedby"] = this.messageId } + // FIXME: placeholder usage is discouraged, we need to find a better UX concept + if (this.defaultValue) { + additionalAttrs["placeholder"] = this.defaultValue + } return { ...this.$attrs, ...additionalAttrs } }, ariaInvalid() { @@ -199,11 +223,14 @@ export default { return this.descriptionMessage }, showClearButton() { - return this.clearButtonEnabled && this.value && this.value.length > 0 + return !this.disabled && this.clearButtonEnabled && this.value !== null }, clearButtonAccessibleLabelValue() { return this.clearButtonAccessibleLabel || this.$gettext("Clear input") }, + displayValue() { + return this.value || "" + }, }, methods: { /** @@ -214,9 +241,17 @@ export default { this.$refs.input.focus() }, onClear() { - this.$refs.input.value = "" - this.$refs.input.focus() - this.onInput("") + this.focus() + + this.onInput(null) + this.onChange(null) + }, + onChange(value) { + /** + * Change event + * @type {event} + **/ + this.$emit("change", value) }, onInput(value) { /** @@ -307,6 +342,10 @@ export default { Focus and select input below + +

+ Value: {{ inputValueWithDefault !== null ? inputValueWithDefault : "null" }} +

Messages

@@ -340,6 +379,7 @@ export default { inputValue: 'initial', valueForMessages: '', inputValueForClearing: 'clear me', + inputValueWithDefault: null, } }, computed: {