Skip to content

Allows asterisk in FormControl.Label to be adjustable #4543

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.

Sign up for GitHub

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

Merged
merged 24 commits into from
May 23, 2024
Merged

Allows asterisk in FormControl.Label to be adjustable #4543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
ad64388
Add new props to `FormControl.Label`
TylerJDev Apr 29, 2024
3ab4b1e
Add conditional
TylerJDev Apr 29, 2024
413ac1e
Add changeset
TylerJDev Apr 30, 2024
add9be3
Update docs json
TylerJDev May 1, 2024
fc84578
Add more examples
TylerJDev May 1, 2024
4d31493
Merge branch 'main' into form-control-required-indicator
TylerJDev May 2, 2024
78d7033
chore(deps-dev): bump ejs from 3.1.9 to 3.1.10 (#4549)
dependabot[bot] May 3, 2024
5ee837c
BranchName: Add style for span and add v8 tokens (#4556)
lukasoppermann May 3, 2024
60dc115
refactor(Banner): update region to use a dedicated aria-label (#4539)
joshblack May 3, 2024
58bd238
chore(deps-dev): bump cross-env from 7.0.2 to 7.0.3 (#4561)
dependabot[bot] May 6, 2024
8a33d1a
chore(deps-dev): bump @babel/plugin-transform-modules-commonjs (#4562)
dependabot[bot] May 6, 2024
35d55bd
chore(deps-dev): bump jest-fail-on-console from 3.1.1 to 3.2.0 (#4563)
dependabot[bot] May 6, 2024
5dd5603
feat(FeatureFlags): broaden feature flag type to accept undefined (#4…
joshblack May 6, 2024
f73f461
prevent form submit (#4551)
siddharthkp May 7, 2024
8d4b60d
deprecate title prop on ActionList.Group component on docs (#4544)
broccolinisoup May 7, 2024
dad8950
chore: add hydro analytics to storybook (#4558)
joshblack May 7, 2024
e6a173b
Revert "Revert "Add support for nested submenus to `ActionMenu`"" (#4…
iansan5653 May 7, 2024
5f29736
chore(deps): update typescript to 5.4.5 (#4568)
joshblack May 7, 2024
741a6a5
Use dynamic height and width for dialogs (#4567)
dgreif May 7, 2024
6bb1726
Make asterisk default, update story scenarios
TylerJDev May 7, 2024
5aa4144
Merge branch 'main' into form-control-required-indicator
TylerJDev May 7, 2024
0d0b531
Merge branch 'main' into form-control-required-indicator
TylerJDev May 9, 2024
afe895d
Update packages/react/src/FormControl/FormControl.docs.json
TylerJDev May 23, 2024
be9b4a6
Update packages/react/src/internal/components/InputLabel.tsx
TylerJDev May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/stupid-suns-cheat.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

FormControl: Adds new props to `FormControl.Label`, `requiredText` and `requiredIndicator`
12 changes: 12 additions & 0 deletions packages/react/src/FormControl/FormControl.docs.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,18 @@
"defaultValue": "'label'",
"description": "The label element can be changed to a 'legend' when it's being used to label a fieldset, or a 'span' when it's being used to label an element that is not a form input. For example: when using a FormControl to render a labeled SegementedControl, the label should be a 'span'"
},
{
"name": "requiredText",
"type": "string",
"defaultValue": "'*'",
"description": "The text to display when the field is required"
},
{
"name": "requiredIndicator",
"type": "boolean",
"defaultValue": "true",
"description": "Whether to show or hide the required text in the accessibility tree, the required text is still shown visually."
},
{
"name": "sx",
"type": "SystemStyleObject"
Expand Down
28 changes: 28 additions & 0 deletions packages/react/src/FormControl/FormControl.features.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
Radio,
RadioGroup,
Select,
Text,
TextInput,
TextInputWithTokens,
Textarea,
Expand Down Expand Up @@ -306,3 +307,30 @@ export const DisabledInputs = () => (
</FormControl>
</Box>
)

export const CustomRequired = () => (
<Box sx={{display: 'flex', flexDirection: 'column', gap: '1rem'}}>
<FormControl required={true}>
<FormControl.Label requiredText="(required)">Form Input Label</FormControl.Label>
<FormControl.Caption>This is a form field with a custom required indicator</FormControl.Caption>
<TextInput />
</FormControl>

<Text sx={{fontSize: 1}}>Required fields are marked with an asterisk (*)</Text>
<FormControl required={true}>
<FormControl.Label requiredIndicator={false}>Form Input Label</FormControl.Label>
<FormControl.Caption>
This is a form field with a required indicator that is hidden in the accessibility tree
</FormControl.Caption>
<TextInput />
</FormControl>

<FormControl required={false}>
<FormControl.Label requiredText="(optional)" requiredIndicator={false}>
Form Input Label
</FormControl.Label>
<FormControl.Caption>This is a form field that is marked as optional, it is not required</FormControl.Caption>
<TextInput />
</FormControl>
</Box>
)
8 changes: 7 additions & 1 deletion packages/react/src/FormControl/_FormControlLabel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ export type Props = {
* Whether the label should be visually hidden
*/
visuallyHidden?: boolean
requiredText?: string
requiredIndicator?: boolean
Comment on lines +11 to +12
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm wondering if this might be better handled on <FormControl> itself, as to apply the required indicator settings to all labels that exist within. The main con here is that there could be cases where you'd only want to adjust one of the indicators, not all.

Copy link
Contributor

Choose a reason for hiding this comment

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

It makes sense to me to add this to the FormControl.Label 🤔 Because that's where the indicator is. I could see users wanting to be able to set a default requiredText for an entire form but it looks like there's no Form element so that's not really possible. I guess in that instance an engineer would probably create a custom component which inherits from the FormControl.Label.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I agree! I think if there's truly value here we could come back to it, but for now I think handling it on FormControl.Label is good enough.

id?: string
} & SxProp

const FormControlLabel: React.FC<
React.PropsWithChildren<{htmlFor?: string} & React.ComponentProps<typeof InputLabel> & Props>
> = ({as, children, htmlFor, id, visuallyHidden, sx, ...props}) => {
> = ({as, children, htmlFor, id, visuallyHidden, requiredIndicator = true, requiredText, sx, ...props}) => {
const {disabled, id: formControlId, required} = useFormControlContext()

/**
Expand All @@ -26,6 +28,8 @@ const FormControlLabel: React.FC<
id,
visuallyHidden,
required,
requiredText,
requiredIndicator,
disabled,
sx,
...props,
Expand All @@ -36,6 +40,8 @@ const FormControlLabel: React.FC<
visuallyHidden,
htmlFor: htmlFor || formControlId,
required,
requiredText,
requiredIndicator,
disabled,
sx,
...props,
Expand Down
8 changes: 6 additions & 2 deletions packages/react/src/internal/components/InputLabel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import VisuallyHidden from '../../_VisuallyHidden'
type BaseProps = SxProp & {
disabled?: boolean
required?: boolean
requiredText?: string
requiredIndicator?: boolean
visuallyHidden?: boolean
id?: string
}
Expand All @@ -28,6 +30,8 @@ const InputLabel: React.FC<React.PropsWithChildren<Props>> = ({
htmlFor,
id,
required,
requiredText,
requiredIndicator,
visuallyHidden,
sx,
as = 'label',
Expand All @@ -52,10 +56,10 @@ const InputLabel: React.FC<React.PropsWithChildren<Props>> = ({
}}
{...props}
>
{required ? (
{required || requiredText ? (
<Box display="flex" as="span">
<Box mr={1}>{children}</Box>
<span>*</span>
<span aria-hidden={requiredIndicator ? undefined : true}>{requiredText ?? '*'}</span>
</Box>
) : (
children
Expand Down