-
Notifications
You must be signed in to change notification settings - Fork 352
Feat: add required indicators to form fields #2815
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
Changes from 14 commits
5e34210
24e4b72
8a59a91
d50dc81
d3ae97d
5782ada
b5de823
bc29a5a
837db00
7e1e16b
1457319
5e8fdac
05bfe9d
c3b8b91
40c2d35
22f279f
d253426
527be2d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| "@bigcommerce/catalyst-core": minor | ||
| --- | ||
|
|
||
| Add default optional text to form input labels for inputs that are not required. | ||
|
|
||
| ## Migration | ||
|
|
||
| The new required props are optional, so they are backwards compatible. However, this does mean that the `(optional)` text will now show up on fields that aren't explicitly marked as required by passing the required prop to the Label component. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -607,5 +607,8 @@ | |
| } | ||
| } | ||
| } | ||
| }, | ||
| "Form": { | ||
| "optional": "optional" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,6 +120,7 @@ export function Checkbox({ | |
| id={id !== undefined ? `${id}-label` : `${generatedId}-label`} | ||
| > | ||
| {label} | ||
| {!props.required && <span className="ml-1 normal-case">(optional)</span>} | ||
|
||
| </LabelPrimitive.Root> | ||
| )} | ||
| </div> | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍹Should punctuation e.g.,
(,)be included innext-intlstrings? Wondering if other languages use a different convention.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wondered about that. Not that I know of. I also wouldn't think the automation we have set up would have an automatic/appropriate translation if that was the case. I wouldn't think our automatic translations would understand the use case just based on the text enough to make a decision?