feat(Datalist): implement full page Basic demo to match HTML #9048

[Kells512]

What: #8982
Adding to the demo section of the DataList component, very similar to the Card View demo but replacing the Card content with DataList content.

[patternfly-build]

Preview: https://patternfly-react-pr-9087.surge.sh

A11y report: https://patternfly-react-pr-9087-a11y.surge.sh

[nicolethoen]

This demo name needs to be updated to match the HTML demo's name

[nicolethoen]

OverflowMenuDropdownItem is also missing from imports

[nicolethoen]

This filtered variable, and the icons variable defined on line 970 are not used anywhere. you can remove them.

[nicolethoen]

@Kells512 I see you made the updates, but for some reason I am not sure, the generated preview link was not updated. Could you try rebasing this PR and force pushing up to this branch - that could fix it and let me review it again. If you want to schedule a video meeting to walk through the rebasing with me, let me know.

[Kells512]

@nicolethoen I think it could be helpful to do it in a meeting, if you are available

[nicolethoen]

Everything looks like it's working well! Now you can just delete a lot of unused code to clean it up. I think you'll need very few state variables ultimately, because your datalist is not interactable, and the toolbar really contains 'fake' filter controls. There is also a lot of code still in this demo related to fetching and displaying card data (which was used in the card view demo you borrowed this from, but is not needed in your demo).
:)

[nicolethoen]

I think you could replace {this.buildFilterDropdown()} with the following:

<MenuToggle>
  Filter by creator name
</MenuToggle>

then you can delete the buildFilterDropdown function as well as all the state variables and functions that were built for it.

• isLowerToolbarDropdownOpen (from state)
• filters (from state)
• this.onDelete
• this.onToolbarDropdownToggle
• this.onNameSelect

[nicolethoen]

I think you could replace {this.buildSelectDropdown()} with the following:

<MenuToggle 
 aria-label="Select cards"
 splitButtonOptions={{
        items: [
                <MenuToggleCheckbox
                  id="split-dropdown-checkbox"
                  aria-label={'Select all cards'}
                />
              ]
    }}
></MenuToggle>

then you can delete the buildSelectDropdown function as well as all the state variables and functions that were built for it.

• splitButtonDropdownIsOpen (from state)
• selectedItems (from state)
• areAllSelected (from state)
• this.onSplitButtonSelect
• this.onSplitButtonToggle

[nicolethoen]

This onCardKebabDropdownSelect and the onCardKebabDropdownToggle are not used anywhere.

[nicolethoen]

This fetch and the following componentDidMount functions are no longer needed.

[nicolethoen]

And I think that will mean that many of the Select or selections related methods might not be needed anymore as well. There is no concept of selecting a DataList row in your demo.

[Kells512]

It seemed a lot of the code could be taken out once i did that, so I took out a lot but can put it back in if there is any issue

[nicolethoen]

The Dropdown in this OverflowMenuControl can be replaced with:

<MenuToggle
     aria-label="Toolbar kebab overflow menu"
     variant="plain"
>
    <EllipsisVIcon />
</MenuToggle>

And then other things you can delete once you've done that which were related to the Dropdown and not needed anymore:

• this.onToolbarKebabDropdownSelect
• isLowerToolbarKebabDropdownOpen (state)
• this.onToolbarKebabDropdownToggle

[nicolethoen]

The demo itself still looks basically perfect! Just a little bit more clean up you can do to unused code.

Quick tip: I'm catching most of this unused code by clicking this button on the workspace, below your demo:

The code sandbox calls out all unused variables and imports.

[nicolethoen]

I believe the corresponding demo in the HTML is simply titled Basic, not Data List view

[nicolethoen]

Badge, DropdownItem, Dropdown, ToolbarFilter, Select are all now unused imports.

[nicolethoen]

perPage is not used by this function anymore. I wonder if it's still needed in the state at all?

[nicolethoen]

In the codesandbox console, I'm seeing Warning: Each child in a list should have a unique "key" prop. and i think it's being generated by the fact that the items in this array need a unique key. In this case, you should be able to simply add key="split-dropdown-checkbox" to the MenuToggleCheckbox.

[nicolethoen]

rather than use all these unicode \xa0 characters, you can add spaceItems={{ default: "spaceItemsSm" }} to the wrapping Flex component.

[Kells512]

I applied that comment!

[kmcfaul]

This key should be moved up to the MenuToggleCheckbox, its being applied to MenuToggle atm.

Everything else lgtm

[andrew-ronaldson]

Looks good, thanks!

[tlabaj]

why is this needed?

[mcoker]

Just a few nits that would be nice to update. There are some other differences, like usage/spacing in flex layouts, and bottom pagination, but I don't know how important any of that is to match core exactly.

[mcoker]

the main text on each row doesn't need to be small, the second line/sub-text should be the only thing that's small.

Suggested change

	<Text component={TextVariants.small}>patternfly</Text>
	<Text component={TextVariants.p}>patternfly</Text>

[mcoker]

Can we just use regular links here without using <Button />? If we need to use the button component, I think we'd want to do add component="a" href="https://pf4.patternfly.org/", though I really think we should update this to something like this (whether it's a normal link or a button component)

Suggested change

	Working repo for PatternFly 4{' '}
	<Button variant="link" isInline>
	https://pf4.patternfly.org/
	</Button>
	Working repo for
	<Button variant="link" isInline component="a" href="http://www.patternfly.org/">
	PatternFly
	</Button>

[mcoker]

Looks like these are missing spaces

Suggested change

	<CodeBranchIcon />5
	</FlexItem>
	<FlexItem>
	<CodeIcon />9
	<CodeBranchIcon /> 5
	</FlexItem>
	<FlexItem>
	<CodeIcon /> 9

[tlabaj]

Can this demo be moved above the Expandable demo?

[tlabaj]

WE are in t he process of converting all our Demo's to typescript. Can we convert this one as well.