Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add explanation of asterisk for required fields to example forms and documentation #820

Merged
merged 13 commits into from
Nov 22, 2024
Merged
9 changes: 7 additions & 2 deletions apps/docs/content/components/FormControl.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ description: Use the form control component to display form inputs alongside lab
import InlineCode from '@primer/gatsby-theme-doctocat/src/components/inline-code'
import {PropTableValues} from '../../src/components'
import {Label} from '@primer/react'
import {Heading} from '@primer/react-brand'
import {Heading, Text} from '@primer/react-brand'
import {SearchIcon} from '@primer/octicons-react'
import {Link} from 'gatsby'

Expand Down Expand Up @@ -83,6 +83,9 @@ An example of a typical layout composed using `FormControl`:
paddingBottom: 3,
}}
>
<Text as="p" variant="muted" size="100">
All fields marked with an asterisk (*) are required
</Text>
<Container
sx={{
display: 'grid',
Expand Down Expand Up @@ -258,7 +261,7 @@ const App = () => {
}

return (
<FormControl validationStatus={validationState}>
<FormControl validationStatus={validationState} fullWidth>
<FormControl.Label>GitHub handle</FormControl.Label>
<TextInput onChange={handleChange} value={value} fullWidth />
{validationState && validationState === 'error' && (
Expand Down Expand Up @@ -328,6 +331,8 @@ Pass the `fullWidth` prop to `FormControl` to provide additional behavior and st

Pass the `required` prop to `FormControl` to provide additional behavior and state context to all `children`, rather than the input only.

When marking a field as required, it is recommended to also provide a corresponding message at the start of the form informing the user that "_all fields marked with an asterisk (\*) are required_".
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Could we also update the form example further up the page please?

Screenshot 2024-11-15 at 16 26 11

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good spot, thanks.

I'd searched for all <form> tags but looks like that was missing one. I've wrapped it in one now and made the required change 👍

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @joshfarrant.

@simmonsjenna here's a sample of the required field statement we need Site to add to all form designs going forward. Let us know if you have any further guidance on spacing, positioning or colors. We can fix in a follow up.

Screenshot 2024-11-21 at 10 45 15

Link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rezrah LGTM


```jsx live
<FormControl required>
<FormControl.Label>Name</FormControl.Label>
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions packages/react/src/forms/form.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ export const GitHubEnterprise = args => {
marginBottom: 32,
}}
>
<Text as="p" variant="muted" size="100">
All fields marked with an asterisk (*) are required
</Text>
<div
style={{
display: 'flex',
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading