owncloud / web

:dragon_face: Next generation frontend for ownCloud Infinite Scale
https://owncloud.dev/clients/web/
GNU Affero General Public License v3.0
427 stars 161 forks source link

Streamline props and events for all form components #8857

Open dschmidt opened 3 years ago

dschmidt commented 3 years ago

It would be nice to have a consistent api for showing label and description-message for all form related components:

Component label description-message defaultValue clearable @change Notes
OcTextInput ✔️ ✔️ ❌ (#1636) / ✔️ (placeholder) ❌ / ✔️(clearButtonEnabled) ✔️ placeholder is discouraged
OcCheckbox ✔️ ❌ (#1635) ❌ (#1635) ❌ (#1635)
OcSelect ✔️ (#1570, owncloud/owncloud-design-system#1633) ❌ (#1634) ✔️ (❌ for multiple:true) ❌ (#1634) 💥 vue-select already has a prop called label
possibly related https://github.com/owncloud/owncloud-design-system/issues/1289 -> https://github.com/owncloud/owncloud-design-system/pull/1301
OcTextArea ✔️ ✔️ ❌ / ✔️ (placeholder) ✔️ (https://github.com/owncloud/owncloud-design-system/issues/1544) placeholder is discouraged
~OcAutocomplete~ ✔️ ✔️ deprecated component
OcDatepicker ✔️ ✔️
OcSwitch ✔️/ ❌ ✔️ label is only used for aria-label but not visible
OcRadio ✔️ does a description message make sense here?
maybe not because OcRadio is only one option and not a whole form element

While at it, it could be nice to have a common css class for all labels/description so it's easier to style all at the same time. Maybe labels could be a component that can be used for attaching them to other form elements, e.g. file input elements that have no component in ODS.

edit: I've added the change event and clearable prop (the latter is only supported by oc-select). It could be nice to have a way to clear values, that way we could even differentiate between empty strings and null for strings e.g.

When no value is set, displaying a default value (placeholder for text-input and text-area) could also be great. It needs to be visually obvious that something is not defined by a user but a not-overridden default value.

Personally I would consider label, description-message and @change to be comparatively high-priority, clearable and placeholder could be nice but certainly have a set of problems with them, so they should be discussed by people with more ux knowledge than me ;-)

edit: I've added defaultValue now that placeholder usage is apparently discouraged. placeholder being a bad solution, doesn't invalidate the problem it tries to solve: show a default value. As a consumer of ODS, I should not care about how things are displayed, I just need a consistent API and rely on ODS to display it in a consistent way using up to date usability standards.

dschmidt commented 3 years ago

Added defaultValue to the table.

dschmidt commented 3 years ago

Additionally I would argue it would make sense to make the clear action emit input and change events with null values. That way clearing can be differentiated from entering empty values.

The clear button on OcTextInput currently emits an empty string via input and doesn't emit change at all.