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.

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

Ux-157 FormElement api simplification #663

Closed
wants to merge 19 commits into from
Closed

Ux-157 FormElement api simplification #663

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
1d5a359
initial simplification
tristanjasper Aug 21, 2020
8539d9a
cleanup
tristanjasper Aug 21, 2020
e05071f
added support for timepicker
tristanjasper Aug 21, 2020
de5ee5c
Merge branch 'master' into ux-157-form-element-simplification
tristanjasper Aug 21, 2020
a28ab6f
prop rename
tristanjasper Aug 21, 2020
659f47e
rename for boolean variable
tristanjasper Aug 21, 2020
0e1933b
more performant rendering
tristanjasper Aug 21, 2020
9a33dfa
move component render calls to external file
tristanjasper Aug 26, 2020
3bfbab4
Merge branch 'master' into ux-157-form-element-simplification
tristanjasper Aug 26, 2020
79569f4
change default props
tristanjasper Aug 26, 2020
22ba49b
ensure ariaDescribedby as prop on checkbox and rad
tristanjasper Aug 26, 2020
c4b52d9
update changelog
tristanjasper Aug 26, 2020
6de6997
Merge branch 'master' into ux-157-form-element-simplification
tristanjasper Aug 28, 2020
934aa9e
first pass simplify a11y props
tristanjasper Aug 28, 2020
d3562fe
change to support simpler elements
tristanjasper Sep 1, 2020
1451e3f
cleanup
tristanjasper Sep 1, 2020
3ac7b93
add console warnings for incorrect api
tristanjasper Sep 1, 2020
350731c
Merge branch 'master' into ux-157-form-element-simplification
tristanjasper Sep 1, 2020
7a5c581
Merge branch "master" into ux-157-form-element-simplification
tristanjasper Sep 2, 2020
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
20 changes: 10 additions & 10 deletions packages/Checkbox/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,16 @@ npm install @paprika/checkbox

### Checkbox

| Prop | Type | required | default | Description |
| --------------- | --------------------------------------- | -------- | ----------------- | ---------------------------------------------------------- |
| ariaDescribedBy | string | false | null | Used for aria-describedby on the checkbox input |
| a11yText | string | false | null | Used for aria-label on the checkbox input |
| checkedState | ["checked","indeterminate","unchecked"] | false | "unchecked" | The checkbox state |
| children | node | false | null | Used for label contents |
| isDisabled | bool | false | false | Describe if the checkbox is disabled or not |
| onChange | func | false | () => {} | Callback triggered when the input state is changed |
| size | ShirtSizes.DEFAULT | false | ShirtSizes.MEDIUM | Size provided by parent Group component |
| tabIndex | [number,string] | false | 0 | Value for tabindex attribute to override the default of 0. |
| Prop | Type | required | default | Description |
| ------------------ | --------------------------------------- | -------- | ----------------- | ---------------------------------------------------------- |
| a11yDescribedByIds | string | false | null | Used for aria-describedby on the checkbox input |
| a11yText | string | false | null | Used for aria-label on the checkbox input |
| checkedState | ["checked","indeterminate","unchecked"] | false | "unchecked" | The checkbox state |
| children | node | false | null | Used for label contents |
| isDisabled | bool | false | false | Describe if the checkbox is disabled or not |
| onChange | func | false | () => {} | Callback triggered when the input state is changed |
| size | ShirtSizes.DEFAULT | false | ShirtSizes.MEDIUM | Size provided by parent Group component |
| tabIndex | [number,string] | false | 0 | Value for tabindex attribute to override the default of 0. |

<!-- autogenerated don't modify -->
<!-- content -->
Expand Down
8 changes: 4 additions & 4 deletions packages/Checkbox/src/Checkbox.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const noop = () => {};

const propTypes = {
/** Used for aria-describedby on the checkbox input */
ariaDescribedBy: PropTypes.string,
a11yDescribedByIds: PropTypes.string,
/** Used for aria-label on the checkbox input */
a11yText: PropTypes.string,
/** The checkbox state */
Expand All @@ -37,7 +37,7 @@ const propTypes = {

const defaultProps = {
a11yText: null,
ariaDescribedBy: null,
a11yDescribedByIds: null,
checkedState: checkboxStates.UNCHECKED,
children: null,
isDisabled: false,
Expand All @@ -54,7 +54,7 @@ const Checkbox = props => {
checkedState,
size,
onChange,
ariaDescribedBy,
a11yDescribedByIds,
tabIndex,
...moreProps
} = props;
Expand All @@ -80,7 +80,7 @@ const Checkbox = props => {
};

const inputProps = {
"aria-describedby": ariaDescribedBy,
"aria-describedby": a11yDescribedByIds,
checked: checkedState === CHECKED,
disabled: isDisabled,
id: checkboxId,
Expand Down
25 changes: 13 additions & 12 deletions packages/DatePicker/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,19 @@ npm install @paprika/date-picker

### DatePicker

| Prop | Type | required | default | Description |
| ----------- | ---------- | -------- | ------------ | ------------------------------------------------------------------------------------------------------------ |
| children | node | false | null | |
| dateFormat | string | false | "MM/DD/YYYY" | Date format used while entering and parsing user input. |
| date | instanceOf | false | null | Selected date in moment object. |
| humanFormat | string | false | undefined | Date format used while displaying date. It should be human-friendly and spelled out, default is MMMM DD,YYYY |
| id | string | false | null | ID for the `<input>`. |
| isDisabled | bool | false | false | Should be disabled or not, default is false. |
| isReadOnly | bool | false | false | Should be read-only or not, default is false. |
| onChange | func | true | - | Callback when date is selected or input. |
| onError | func | false | () => {} | Internal errors callback |
| hasError | bool | false | false | If there is an external error or not. |
| Prop | Type | required | default | Description |
| ------------------ | ---------- | -------- | ------------ | ------------------------------------------------------------------------------------------------------------ |
| a11yDescribedByIds | string | false | null | Used for aria-describedby on the date input |
| children | node | false | null | |
| dateFormat | string | false | "MM/DD/YYYY" | Date format used while entering and parsing user input. |
| date | instanceOf | false | null | Selected date in moment object. |
| humanFormat | string | false | undefined | Date format used while displaying date. It should be human-friendly and spelled out, default is MMMM DD,YYYY |
| id | string | false | null | ID for the `<input>`. |
| isDisabled | bool | false | false | Should be disabled or not, default is false. |
| isReadOnly | bool | false | false | Should be read-only or not, default is false. |
| onChange | func | true | - | Callback when date is selected or input. |
| onError | func | false | () => {} | Internal errors callback |
| hasError | bool | false | false | If there is an external error or not. |

<!-- autogenerated don't modify -->
<!-- content -->
Expand Down
19 changes: 18 additions & 1 deletion packages/DatePicker/src/DatePicker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import DatePickerPopoverPropsCollector from "./components/DatePickerPopoverProps
import { calendarPopoverStyles } from "./DatePicker.styles";

const propTypes = {
/** Used for aria-describedby on the date input */
a11yDescribedByIds: PropTypes.string,

children: PropTypes.node,

/** Date format used while entering and parsing user input. */
Expand Down Expand Up @@ -55,11 +58,24 @@ const defaultProps = {
isDisabled: false,
isReadOnly: false,
onError: () => {},
a11yDescribedByIds: null,
};

function DatePicker(props) {
// Props
const { children, dateFormat, date, humanFormat, id, isDisabled, isReadOnly, onChange, onError, hasError } = props;
const {
children,
dateFormat,
date,
humanFormat,
id,
isDisabled,
isReadOnly,
onChange,
onError,
hasError,
a11yDescribedByIds,
} = props;

// State
const [possibleDate, setPossibleDate] = React.useState(null);
Expand Down Expand Up @@ -144,6 +160,7 @@ function DatePicker(props) {
denyConfirmation={() => isElementContainsFocus(calendarRef.current)}
{...extendedInputProps}
hasError={hasErrorValue}
a11yDescribedByIds={a11yDescribedByIds}
/>

<Popover.Content>
Expand Down
6 changes: 6 additions & 0 deletions packages/FormElement/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
-- Changed the spacing between label, content and instructions

[@tristanjasper](https://github.com/tristanjasper).

## [Unreleased]

### Changed

- Simplified formElement api for consumers - [@tristanjasper](https://github.com/tristanjasper).
61 changes: 26 additions & 35 deletions packages/FormElement/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,13 @@ npm install @paprika/form-element

### FormElement.Content

| Prop | Type | required | default | Description |
| --------------- | ----------- | -------- | ------- | -------------------------------------------- |
| children | [func,node] | true | - | |
| idForLabel | string | false | null | Sets id for label |
| refLabel | custom | false | null | |
| ariaDescribedBy | string | false | null | Used for aria-describedby on the FormElement |
| Prop | Type | required | default | Description |
| ------------------------- | ----------- | -------- | ------- | ------------------------------------------------------------------------------------ |
| children | [func,node] | true | - | |
| idForLabel | string | false | null | Sets id for label |
| refLabel | custom | false | null | |
| a11yDescribedByIds | string | false | null | Used for aria-describedby on the FormElement |
| wrapperA11yDescribedByIds | string | false | null | Used when form Elements are nested and is automatically appended to aria-describedby |

### FormElement.Description

Expand Down Expand Up @@ -94,20 +95,16 @@ import Input from "@paprika/input";

<FormElement label="Name">
<FormElement.Content>
{({ idForLabel, ariaDescribedBy }) => (
<Input
id={idForLabel}
onChange={handleChange}
value={value}
placeholder="Form placeholder"
aria-describedby={ariaDescribedBy}
aria-required={hasRequiredLabel}
hasError={false}
isDisabled={isDisabled}
isReadOnly={isReadOnly}
size={size}
/>
)}
<Input
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a breaking change or our FormElements still support the renderFunction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be a breaking change. I don't think we should encourage using the renderFunction as an acceptable api moving forward

onChange={handleChange}
value={value}
placeholder="Form placeholder"
aria-required={hasRequiredLabel}
hasError={false}
isDisabled={isDisabled}
isReadOnly={isReadOnly}
size={size}
/>
</FormElement.Content>
</FormElement>;
```
Expand All @@ -120,11 +117,9 @@ import FormElement from "@paprika/form-element";

<FormElement label="Name">
<FormElement.Content>
{({ idForLabel, ariaDescribedBy }) => (
<DatePicker onError={() => {}} hasError={Boolean(errorText.length)} id={idForLabel} onChange={() => {}}>
<DatePicker.Input aria-describedby={ariaDescribedBy} />
</DatePicker>
)}
<DatePicker onError={() => {}} hasError={Boolean(errorText.length)} onChange={() => {}}>
<DatePicker.Input />
</DatePicker>
</FormElement.Content>
</FormElement>;
```
Expand All @@ -136,16 +131,12 @@ import FormElement from "@paprika/form-element";

<FormElement hasRequiredLabel={hasRequiredLabel} label="Name">
<FormElement.Content>
{({ idForLabel, ariaDescribedBy }) => (
<input
aria-required={hasRequiredLabel}
aria-describedby={ariaDescribedBy}
aria-invalid={Boolean(errorText.length)}
disabled={isDisabled}
id={idForLabel}
readOnly={isReadOnly}
/>
)}
<input
aria-required={hasRequiredLabel}
aria-invalid={Boolean(errorText.length)}
disabled={isDisabled}
readOnly={isReadOnly}
/>
</FormElement.Content>
</FormElement>;
```
Expand Down
2 changes: 1 addition & 1 deletion packages/FormElement/src/FormElement.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ function FormElement(props) {
return getClonedElement(subComponentDisplayNames.Content, {
idForLabel,
refLabel,
ariaDescribedBy: `${ariaErrorId} ${ariaInstructionsId} ${ariaDescriptionId}`,
a11yDescribedByIds: `${ariaErrorId} ${ariaInstructionsId} ${ariaDescriptionId}`,
});
}

Expand Down
Loading