-
Notifications
You must be signed in to change notification settings - Fork 646
Deprecate ChoiceFieldset and ChoiceInputField #1900
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 3 commits
e55d62c
8276358
d88aad9
210dfdc
74e77e0
3a48ea2
da2d956
1c545f4
5d3a36d
da4fc21
f2dc311
f521c8b
9776b9b
6888e5c
f117df8
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,116 @@ | ||||||||||
| --- | ||||||||||
| '@primer/react': major | ||||||||||
| --- | ||||||||||
|
|
||||||||||
|  | ||||||||||
|
|
||||||||||
| The `CheckboxGroup` and `RadioGroup` components will be used to deprecate the `ChoiceFieldset` component. The deprecation of `ChoiceFieldset` also allows us to deprecate `ChoiceInputField`. | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's a quick one, can we add the heading in? Happy to do it in the final changelog too, so no biggie |
||||||||||
|
|
||||||||||
| `CheckboxGroup` and `RadioGroup` have the ability to render contextual content with your fieldset: labels, validation statuses, captions. It also handles the ARIA attributes that make the form controls accessible to assistive technology. | ||||||||||
|
|
||||||||||
| <table> | ||||||||||
| <tr> | ||||||||||
| <th> Before </th> <th> After </th> | ||||||||||
|
||||||||||
| <th> Before </th> <th> After </th> | |
| <th> Before (v34) </th> <th> After (v35)</th> |
Outdated
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.
The before / after is overflowing quite a lot.. is it possible to add more line breaks to the before snippet so it brings after more into view?
| <ChoiceFieldset.Legend>Prefered Primer component interface</ChoiceFieldset.Legend> | |
| <ChoiceFieldset.Legend> | |
| Prefered Primer component interface | |
| </ChoiceFieldset.Legend> |
Outdated
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.
FYI typo on preferred
Outdated
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.
small typo
| <CheckboxGroup.Label>Prefered Primer component interface</CheckboxGroup.Label> | |
| <CheckboxGroup.Label>Preferred Primer component interface</CheckboxGroup.Label> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,14 +8,6 @@ export type {ThemeProviderProps} from './ThemeProvider' | |
| // Layout | ||
| export {default as Box} from './Box' | ||
| export type {BoxProps} from './Box' | ||
| export {PageLayout} from './PageLayout' | ||
| export type { | ||
| PageLayoutProps, | ||
| PageLayoutHeaderProps, | ||
| PageLayoutContentProps, | ||
| PageLayoutPaneProps, | ||
| PageLayoutFooterProps | ||
| } from './PageLayout' | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems unintentional? |
||
|
|
||
| // Hooks | ||
| export {default as useDetails} from './hooks/useDetails' | ||
|
|
@@ -74,12 +66,10 @@ export type { | |
| } from './Button' | ||
| export {default as Caret} from './Caret' | ||
| export type {CaretProps} from './Caret' | ||
| export {default as ChoiceInputField} from './ChoiceInputField' | ||
| export {default as CircleBadge} from './CircleBadge' | ||
| export type {CircleBadgeProps, CircleBadgeIconProps} from './CircleBadge' | ||
| export {default as CircleOcticon} from './CircleOcticon' | ||
| export type {CircleOcticonProps} from './CircleOcticon' | ||
| export {default as ChoiceFieldset, Item} from './ChoiceFieldset' | ||
| export {default as CheckboxGroup} from './CheckboxGroup' | ||
| export {default as CounterLabel} from './CounterLabel' | ||
| export type {CounterLabelProps} from './CounterLabel' | ||
|
|
@@ -127,7 +117,6 @@ export type {PopoverProps, PopoverContentProps} from './Popover' | |
| export {default as ProgressBar} from './ProgressBar' | ||
| export type {ProgressBarProps} from './ProgressBar' | ||
| export {default as RadioGroup} from './RadioGroup' | ||
| export {default as SelectMenu} from './SelectMenu' | ||
| export {default as Select} from './Select' | ||
| export {SelectPanel} from './SelectPanel' | ||
| export type {SelectPanelProps} from './SelectPanel' | ||
|
|
||
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.
If we go with these banners in the change logs*, can we change this to a more descriptive caption here please?
Also, wondering what you think about switching the labels and group headings to be more product-oriented in the banner, like they are in code snippets?
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.
Ok, so I'll remove the banner