-
Notifications
You must be signed in to change notification settings - Fork 9
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
Pd 12487 collapsible checklist #123
Changes from all commits
52f57fb
61fd8ea
2192f4d
df065fe
cd91187
701e73b
99dcd15
8b06944
46636df
33430a8
6ed294d
c32e77a
accaf6f
69c362a
55df901
1e1dd29
11b2278
93be5ea
c8f6943
6b0236f
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,47 @@ | ||
## CollapsibleChecklists | ||
|
||
A controlled component that has checkable groups of checkable items. | ||
Checking the parent checks all of the children. | ||
|
||
### Installation | ||
|
||
`> npm install --save @paprika/collapsible-checklists` | ||
or | ||
`> yarn add @paprika/collapsible-checklists` | ||
|
||
### Usage | ||
|
||
```js | ||
import CollapsibleChecklists from "@paprika/collapsible-checklists"; | ||
|
||
const YourComponent = () => { | ||
return ( | ||
<CollapsibleChecklists | ||
onChange={() => { | ||
/*...*/ | ||
}} | ||
> | ||
<CollapsibleChecklists.Heading>Flowers</CollapsibleChecklists.Heading> | ||
|
||
<CollapsibleChecklists.Group title="Roses"> | ||
<CollapsibleChecklists.Item isChecked>Damask</CollapsibleChecklists.Item> | ||
<CollapsibleChecklists.Item isChecked>Eden</CollapsibleChecklists.Item> | ||
<CollapsibleChecklists.Item isChecked>Lady Banks</CollapsibleChecklists.Item> | ||
</CollapsibleChecklists.Group> | ||
|
||
<CollapsibleChecklists.Group title="Irises"> | ||
<CollapsibleChecklists.Item isChecked>Siberian</CollapsibleChecklists.Item> | ||
<CollapsibleChecklists.Item>Yellow</CollapsibleChecklists.Item> | ||
<CollapsibleChecklists.Item isDisabled>Bearded</CollapsibleChecklists.Item> | ||
<CollapsibleChecklists.Item>Netted</CollapsibleChecklists.Item> | ||
</CollapsibleChecklists.Group> | ||
</CollapsibleChecklists> | ||
); | ||
}; | ||
|
||
export default YourComponent; | ||
``` | ||
|
||
### Props | ||
|
||
- `onChange` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
{ | ||
"name": "@paprika/collapsible-checklists", | ||
"version": "0.1.0", | ||
"description": "CollapsibleChecklists component", | ||
"author": "@paprika", | ||
"license": "MIT", | ||
"main": "lib/index.js", | ||
"repository": { | ||
"type": "git", | ||
"url": "https://github.com/acl-services/paprika/tree/master/packages/CollapsibleChecklists" | ||
}, | ||
"publishConfig": { | ||
"access": "public" | ||
}, | ||
"dependencies": { | ||
"@babel/runtime-corejs2": "^7.3.1", | ||
"@paprika/collapsible": "^0.1.0", | ||
"@paprika/stylers": "^0.1.1", | ||
"@paprika/tokens": "^0.1.1", | ||
"prop-types": "^15.7.2" | ||
}, | ||
"peerDependencies": { | ||
"react": "^16.8.0", | ||
"react-dom": "^16.8.0", | ||
"styled-components": "^4.2.0" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import React from "react"; | ||
import PropTypes from "prop-types"; | ||
import Heading from "./components/Heading"; | ||
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. should import paprika components instead of relative paths 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. oh its not a paprika Heading, its my own 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. Wondering if perhaps we should try to not call our own components the same as paprika component names in general to avoid that confusion |
||
import Group from "./components/Group"; | ||
import Item from "./components/Item"; | ||
|
||
const propTypes = { | ||
children: PropTypes.node.isRequired, | ||
onChange: PropTypes.func.isRequired, | ||
}; | ||
|
||
const defaultProps = {}; | ||
|
||
const CollapsibleChecklists = props => { | ||
const { children, onChange } = props; | ||
const modifiedChildren = []; | ||
|
||
React.Children.forEach(children, (child, index) => { | ||
switch (child.type.displayName) { | ||
case Group.displayName: | ||
modifiedChildren.push(React.cloneElement(child, { key: `group${index}`, onChange })); // eslint-disable-line react/no-array-index-key | ||
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. That ESLint rule speaks the truth. Even if you're not explicitly reordering items a la |
||
break; | ||
default: | ||
modifiedChildren.push(child); | ||
} | ||
}); | ||
|
||
return <React.Fragment>{modifiedChildren}</React.Fragment>; | ||
}; | ||
|
||
CollapsibleChecklists.displayName = "CollapsibleChecklists"; | ||
CollapsibleChecklists.propTypes = propTypes; | ||
CollapsibleChecklists.defaultProps = defaultProps; | ||
|
||
CollapsibleChecklists.Heading = Heading; | ||
CollapsibleChecklists.Group = Group; | ||
CollapsibleChecklists.Item = Item; | ||
|
||
export default CollapsibleChecklists; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,129 @@ | ||||||
import React from "react"; | ||||||
import PropTypes from "prop-types"; | ||||||
import Collapsible from "@paprika/collapsible"; | ||||||
import Item from "../Item"; | ||||||
import groupStyles from "./Group.styles"; | ||||||
|
||||||
const propTypes = { | ||||||
children: PropTypes.node, // probably an array of "Items", but could be a Spinner or anything else | ||||||
isDisabled: PropTypes.bool, | ||||||
onChange: PropTypes.func, | ||||||
onExpand: PropTypes.func, | ||||||
title: PropTypes.node.isRequired, | ||||||
}; | ||||||
|
||||||
const defaultProps = { | ||||||
children: [], | ||||||
isDisabled: false, | ||||||
onChange: () => {}, | ||||||
onExpand: () => {}, | ||||||
}; | ||||||
|
||||||
function useIsIndeterminate(checkboxRef) { | ||||||
const [, setIsIndeterminate] = React.useState(null); | ||||||
React.useEffect(() => { | ||||||
setIsIndeterminate(checkboxRef.current.indeterminate); | ||||||
}, [checkboxRef.current.indeterminate]); | ||||||
} | ||||||
|
||||||
function Group(props) { | ||||||
const { children, isDisabled, onChange, onExpand, title } = props; | ||||||
const [isCollapsed, setIsCollapsed] = React.useState(true); | ||||||
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. Perhaps in the future we might want to have an initial value for this |
||||||
const checkboxRef = React.useRef({}); | ||||||
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.
Suggested change
is the recommended way to initialize refs for DOM components 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. @alexzherdev this does not work as I am calling |
||||||
useIsIndeterminate(checkboxRef); | ||||||
let allAreChecked = React.Children.count(props.children) > 0; | ||||||
let noneAreChecked = true; | ||||||
|
||||||
React.Children.forEach(children, child => { | ||||||
if (child.props.isChecked) { | ||||||
noneAreChecked = false; | ||||||
} else { | ||||||
allAreChecked = false; | ||||||
} | ||||||
}); | ||||||
|
||||||
if (allAreChecked || noneAreChecked) { | ||||||
checkboxRef.current.indeterminate = false; | ||||||
} else if (!allAreChecked && !noneAreChecked) { | ||||||
checkboxRef.current.indeterminate = true; | ||||||
} | ||||||
|
||||||
function toggleChildren() { | ||||||
const childItems = []; | ||||||
React.Children.forEach(children, child => { | ||||||
if (child.type === Item) { | ||||||
childItems.push(child); | ||||||
} | ||||||
}); | ||||||
|
||||||
if (childItems.length === 0) { | ||||||
return; | ||||||
} | ||||||
|
||||||
const allChildItemsAreChecked = | ||||||
childItems.filter(childItem => childItem.props.isChecked || childItem.props.isDisabled).length === | ||||||
childItems.length; | ||||||
|
||||||
let childItemsToChange = []; | ||||||
if (allChildItemsAreChecked) { | ||||||
childItemsToChange = childItems.filter(childItem => !childItem.props.isDisabled); | ||||||
} else { | ||||||
childItemsToChange = childItems.filter(childItem => !childItem.props.isChecked && !childItem.props.isDisabled); | ||||||
} | ||||||
|
||||||
onChange(childItemsToChange); | ||||||
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. I'm interested in hearing others' feedback about this. While discussing the API for We might want to require some kind of 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. Since we expect an array, maybe just the 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. The idea is that you always received the selectedIndexes as the first argument, and we could pass the full list of elements as second argument so you could do: function handleChange(indexes, list) {
indexes.forEach(index => console.log(list[index].props.value))
} where value can be any property they want to access, can be id, data, etc... of course, if we want to get the ones that haven't been selected we would need to use filter instead of map or forEach. 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. We only have a single For the snippet above, seems like the list is redundant? The consumer is the one passing in those "values" in the first place, so they don't really need to be given them back. They already have some kind of state that they can iterate over. I suggest creating a story that would better reflect the real world usage of this component (i.e. resembling the feature that will need it). That should make it easier to design. 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. The consumer can compare on anything... I chose to compare by 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. I see, I'm -1 on this. |
||||||
} | ||||||
|
||||||
/* eslint-disable jsx-a11y/label-has-associated-control */ | ||||||
const label = ( | ||||||
<React.Fragment> | ||||||
<label> | ||||||
<input | ||||||
ref={checkboxRef} | ||||||
checked={allAreChecked} | ||||||
type="checkbox" | ||||||
disabled={isDisabled} | ||||||
onChange={toggleChildren} | ||||||
/> | ||||||
{title} | ||||||
</label> | ||||||
</React.Fragment> | ||||||
); | ||||||
/* eslint-enable jsx-a11y/label-has-associated-control */ | ||||||
|
||||||
const modifiedChildren = []; | ||||||
React.Children.forEach(children, (child, index) => { | ||||||
const newProps = { | ||||||
key: `${title}-child-${index}`, | ||||||
}; | ||||||
|
||||||
if (child.type.displayName === Item.displayName) { | ||||||
newProps.onChange = () => onChange([child]); | ||||||
} | ||||||
|
||||||
modifiedChildren.push(React.cloneElement(child, newProps)); | ||||||
}); | ||||||
|
||||||
return ( | ||||||
<Collapsible | ||||||
css={groupStyles} | ||||||
hasOnlyIconToggle | ||||||
isCollapsed={isCollapsed} | ||||||
isDisabled={isDisabled} | ||||||
label={label} | ||||||
onClick={() => { | ||||||
if (isCollapsed && onExpand) { | ||||||
onExpand(); | ||||||
} | ||||||
setIsCollapsed(!isCollapsed); | ||||||
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.
Suggested change
https://reactjs.org/docs/hooks-reference.html#functional-updates |
||||||
}} | ||||||
> | ||||||
{modifiedChildren} | ||||||
</Collapsible> | ||||||
); | ||||||
} | ||||||
|
||||||
Group.displayName = "Group"; | ||||||
Group.propTypes = propTypes; | ||||||
Group.defaultProps = defaultProps; | ||||||
export default Group; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import tokens from "@paprika/tokens"; | ||
import { css } from "styled-components"; | ||
import { disabledStyle } from "../helpers.styles"; | ||
|
||
const groupStyles = css` | ||
${({ isDisabled }) => { | ||
return ` | ||
[data-pka-anchor="collapsible.heading"] { | ||
align-items: center; | ||
background-color: ${tokens.color.cremeLighten5}; | ||
border-bottom: 1px solid ${tokens.color.blackLighten60}; | ||
display: flex; | ||
padding: ${tokens.spaceSm}; | ||
|
||
input[type="checkbox"] { | ||
margin-right: ${tokens.space}; | ||
} | ||
|
||
${isDisabled ? disabledStyle : ""}; | ||
} | ||
`; | ||
}} | ||
`; | ||
|
||
export default groupStyles; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default } from "./Group"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import React from "react"; | ||
import PropTypes from "prop-types"; | ||
import headingStyles from "./Heading.styles"; | ||
|
||
const propTypes = { | ||
children: PropTypes.node.isRequired, | ||
}; | ||
|
||
const defaultProps = {}; | ||
|
||
const Heading = ({ children }) => <div css={headingStyles}>{children}</div>; | ||
|
||
Heading.displayName = "Heading"; | ||
Heading.propTypes = propTypes; | ||
Heading.defaultProps = defaultProps; | ||
|
||
export default Heading; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import tokens from "@paprika/tokens"; | ||
import { css } from "styled-components"; | ||
import stylers from "@paprika/stylers"; | ||
|
||
const headingStyles = css` | ||
background-color: ${tokens.table.header.backgroundColor}; | ||
border-bottom: 2px solid ${tokens.border.color}; | ||
font-weight: bold; | ||
padding: ${tokens.space} ${tokens.space} ${tokens.space} ${stylers.spacer(2)}; | ||
`; | ||
|
||
export default headingStyles; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default } from "./Heading"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import React from "react"; | ||
import PropTypes from "prop-types"; | ||
import itemStyles from "./Item.styles"; | ||
|
||
const propTypes = { | ||
children: PropTypes.node.isRequired, | ||
isChecked: PropTypes.bool, | ||
isDisabled: PropTypes.bool, | ||
onChange: PropTypes.func, | ||
}; | ||
|
||
const defaultProps = { | ||
isChecked: false, | ||
isDisabled: false, | ||
onChange: () => {}, | ||
}; | ||
|
||
function Item(props) { | ||
const { children, isChecked, isDisabled, onChange } = props; | ||
|
||
/* eslint-disable jsx-a11y/label-has-associated-control */ | ||
return ( | ||
<div css={itemStyles} isDisabled={isDisabled}> | ||
<label> | ||
<input type="checkbox" checked={isChecked} disabled={isDisabled} onChange={onChange} /> | ||
{children} | ||
</label> | ||
</div> | ||
); | ||
/* eslint-enable jsx-a11y/label-has-associated-control */ | ||
} | ||
|
||
Item.displayName = "Item"; | ||
Item.propTypes = propTypes; | ||
Item.defaultProps = defaultProps; | ||
|
||
export default Item; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import stylers from "@paprika/stylers"; | ||
import tokens from "@paprika/tokens"; | ||
import { css } from "styled-components"; | ||
import { disabledStyle } from "../helpers.styles"; | ||
|
||
const itemStyles = css` | ||
${({ isDisabled }) => { | ||
return ` | ||
padding: 2px 0 2px ${stylers.spacer(5)}; | ||
${isDisabled ? disabledStyle : ""}; | ||
|
||
input { | ||
margin-right: ${tokens.space} | ||
} | ||
`; | ||
}} | ||
`; | ||
|
||
export default itemStyles; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default } from "./Item"; |
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 think we should use homepage for this value and url property should point to the repo itself.
read more: #91