feat(CC-batch-5): added batch 5 #11867
Conversation
|
Preview: https://patternfly-react-pr-11867.surge.sh A11y report: https://patternfly-react-pr-11867-a11y.surge.sh |
| status={props.status.type} | ||
| titleText={props.title} | ||
| variant={props.variant} | ||
| headingLevel="h4" |
There was a problem hiding this comment.
we should add a comment here indicating that this may need to be updated for accessibility
maybe link to https://www.w3.org/WAI/tutorials/page-structure/headings/
| 'Extra large': 'xl' | ||
| }), | ||
| // "custom" | "warning" | "success" | "danger" | "info" | ||
| status: figma.enum('Type', { |
There was a problem hiding this comment.
I dont understand how this Type prop is affecting the example code. It doesn't look like it's being passed at all.
| type: undefined | ||
| } | ||
| }), | ||
| isLoading: figma.enum('Type', { Loading: 'Spinner' }) |
There was a problem hiding this comment.
This doesn't appear to be used in the example code below at all
| example: (props) => ( | ||
| // Documentation for ExpandableSection can be found at https://www.patternfly.org/components/expandable-section | ||
| <ExpandableSection | ||
| toggleTextCollapsed={props.toggleTextCollapsed} |
There was a problem hiding this comment.
We should not be combining toggleContent, toggleTextCollapsed, and toggleTextExpanded in the same ExpandableSection.
If toggleContent is provided, it overrides the other two.
I think these ExpandableSection implementations should choose one or the other. it can leave the other option commented out with an explanation...
And onToggle with isExpanded are only required if the user wants to be able to have other side effects when opening/closing the expandable section (such as fetching the data to load in the expanded section only if the user tries to expand).
So those two props can be commented out by default letting them know in a comment that they can override the default expand/collapse behavior by using those two props.
| example: (props) => ( | ||
| // Documentation for ExpandableSection can be found at https://www.patternfly.org/components/expandable-section | ||
| <ExpandableSection | ||
| isExpanded={props.isExpanded} |
There was a problem hiding this comment.
I think this needs to be rewritten based on my comments in the ExpandableSectionTruncate file.
| example: (props) => ( | ||
| // Documentation for ExpandableSection can be found at https://www.patternfly.org/components/expandable-section | ||
| <ExpandableSection | ||
| isExpanded={props.isExpanded} |
There was a problem hiding this comment.
I think this needs to be rewritten based on my comments in the ExpandableSectionTruncate file.
|
|
||
| state: figma.enum('State', { | ||
| Default: 'default', | ||
| 'Drag over': 'drag-over', |
There was a problem hiding this comment.
What is this enum being used for? i don't see any references to these concepts in the example code below
There was a problem hiding this comment.
My thought is that this is tracking different states for file uploading from a design perspective, but the implementation doesn't really change so this may not be necessary to map.
|
|
||
| const actions = ( | ||
| <Dropdown | ||
| isOpen={isOpen} |
There was a problem hiding this comment.
This will not work as written since ReferenceError: isOpen is not defined
I think this dropdown should be updated to something like:
const actions = (
<Dropdown
isOpen={false} // should use an 'isOpen' state variable
onSelect={()=>{}} // need to define an onSelect handler
onOpenChange={(isOpen: boolean) => setIsOpen(isOpen)}
toggle={(toggleRef: React.Ref<MenuToggleElement>) => (
<MenuToggle
ref={toggleRef}
aria-label="Without title example kebab toggle"
variant="plain"
onClick={()=>{}} // should toggle the value of 'isOpen' state variable
isExpanded={false}
icon={"..."} // should use an icon
/>
)}
>
<DropdownList>
<DropdownItem value={0} key="action">
Action
</DropdownItem>
<DropdownItem
value={1}
key="link"
to="#default-link2"
// Prevent the default onClick functionality for example purposes
onClick={(ev: any) => ev.preventDefault()}
>
Link
</DropdownItem>
<DropdownItem value={2} isDisabled key="disabled action">
Disabled Action
</DropdownItem>
<DropdownItem value={3} isDisabled key="disabled link" to="#default-link4">
Disabled Link
</DropdownItem>
<Divider component="li" key="separator" />
<DropdownItem value={4} key="separated action">
Separated Action
</DropdownItem>
<DropdownItem value={5} key="separated link" to="#default-link6" onClick={(ev) => ev.preventDefault()}>
Separated Link
</DropdownItem>
</DropdownList>
</Dropdown>
);
| icon={props.status.icon} | ||
| > | ||
| <EmptyStateBody>{props.body}</EmptyStateBody> | ||
| {/* <EmptyStateFooter>{props.children}</EmptyStateFooter> */} |
There was a problem hiding this comment.
Is this meant to connect to Figma or is this just a comment showing EmptyStateFooter?
| example: (props) => ( | ||
| // Documentation for ExpandableSection can be found at https://www.patternfly.org/components/expandable-section | ||
| <ExpandableSection | ||
| toggleTextCollapsed={props.toggleTextCollapsed} |
|
|
||
| state: figma.enum('State', { | ||
| Default: 'default', | ||
| 'Drag over': 'drag-over', |
There was a problem hiding this comment.
My thought is that this is tracking different states for file uploading from a design perspective, but the implementation doesn't really change so this may not be necessary to map.
| infoText="Accepted file types: JPEG, Doc, PDF, PNG" | ||
| /> | ||
|
|
||
| <MultipleFileUploadStatus |
There was a problem hiding this comment.
MultipleFileUploadStatus is conditionally shown in our examples after a file is uploaded. Unsure if that's too complex to stub out so maybe a comment would be helpful here.
| titleIcon={'<UploadIcon />'} | ||
| titleText="Drag and drop files here" | ||
| titleTextSeparator="or" | ||
| infoText="Accepted file types: JPEG, Doc, PDF, PNG" |
There was a problem hiding this comment.
If we're stubbing out this infoText with defined accepted file types we may also want to add a stubbed dropzoneProps to MultipleFileUpload to match:
dropzoneProps={{ accept: { 'image/jpeg': ['.jpg', '.jpeg'], 'application/msword': ['.doc'], 'application/pdf': ['.pdf'], 'image/png': ['.png'] } }}
| false: undefined | ||
| }), | ||
|
|
||
| actions: figma.instance('Swap button') |
Relates to: #11624
Included components:
Component tracker
Figma preview
Resources: