element-hq / element-hq/compound
Reorganise and cleanup the form elements
- 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.