element-hq / element-hq/compound

Reorganise and cleanup the form elements

Open
#249 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
MDX
Stars
24
Forks
7
PR merge metrics
No merged PRs in 30d

Description

While working on the readonly state on forms and the MFA component, I noticed that the form-related components are weirdly organised and don't always have the best implementation.
Sorry for the "brain-dump" style list, I can open individual issues if needed.

- All forms components should be exported through a `Form` object, so that markup like this is possible:
```tsx
import { Form } from "@vector-im/compound-web";
const Render = () => (


Username



Password


Login

);
```
- The default `` should have sensible default gap and flexbox. `--cpd-space-5x` looks like a good default candidate
- The password control has multiple issues, including:
- [It doesn't stretch properly when in a flex container](https://quenting-mfa-control.compound-web.pages.dev/?path=/story/form--root)
- [It disables autocomplete and autocorrect by default, whilst it should be the responsibility of the parent component to do this](https://github.com/vector-im/compound-web/blob/7f75c26861fcab4209c9f8070dec048c9a4ee272/src/components/ActionControl/ActionControl.tsx#L65-L66)
- [The `StandaloneActionControl` doesn't make sense?](https://github.com/vector-im/compound-web/blob/7f75c26861fcab4209c9f8070dec048c9a4ee272/src/components/ActionControl/ActionControl.tsx#L83-L94)
- [The search component has weird (and probably inaccessible) markup.](https://github.com/vector-im/compound-web/blob/3c165ed1a830000a98104b3f30f8eed98244d5c9/src/components/Search/Search.tsx#L48-L72) It uses a Label while it doesn't have a real one, and doesn't use the radix' control, meaning that it probably doesn't get the accessibility behaviors
- [The form root has default typography](https://github.com/vector-im/compound-web/blob/bca8ee55f95d24fc89bd8a86a2053d8998edce0c/src/components/Form/form.module.css#L23-L26), which I think shouldn't be set there, but rather in the form components directly
- The form inputs font size is 15px, which is just low enough to make iOS zoom on the input. We should really make it 16px. It is actually `0.9375rem` whereas it should really be `1rem`
- [`ValidityState` redefines a component which doesn't add anything, and really should just be a re-export from Radix](https://github.com/vector-im/compound-web/blob/f42628a96529d9b1906091d68ca82dfa99422717/src/components/Form/ValidityState.tsx#L24-L28)
- The `Message` component doesn't implement the "error message" styling. I think we should do distinct `HelpMessage` and `ErrorMessage` components
- All the form components should be in the `form` folder
- There is a mix of radix and non-radix controls:
- `Radio` and `Checkbox` aren't using radix
- `MFAControl`, `ActionControl`, `PasswordControl` are using radix
- I think we should use radix for all of them or none of them (and use `` for example?)
- We should have distinct stories for each form component, organised in a folder, with all the possible states (disabled, readonly, invalid, etc)

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.