PatternFly-React preview: https://patternfly-react-pr-3127.surge.sh

Codecov Report

Merging #3127 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3127      +/-   ##
==========================================
- Coverage   69.03%   69.03%   -0.01%     
==========================================
  Files         859      859              
  Lines       23637    23641       +4     
  Branches     1895     1895              
==========================================
+ Hits        16318    16320       +2     
- Misses       6359     6361       +2     
  Partials      960      960

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e77c613...cc4834b. Read the comment docs.

Added do not merge to wait for other PRs to go in first

@kmcfaul I took a quick peek at this realizing that it still may be WIP. I think it's looking really good. Here are just a few small things I noticed:

• Styling should be consistent with core styling here https://patternfly-next-pr-2287.surge.sh/documentation/core/demos/datatoolbar Note difference in the search field specifically.

• For the search field, typing and pressing enter should have the same effect as clicking the search button.

• When the filter leads to 0 results, the empty state pattern should be used as seen in this demo: https://www.patternfly.org/v4/documentation/core/demos/table-full?component=Table%20-%20empty%20state%20demo

• Is it possible to fix the width of the input group so that you don't get an accordion affect as you select different items? I'm concerned to in the context of a toolbar, items following the filter will move around which is not a great experience. It can be up to the designer to specify a width that will work for the likely content.

All other behavior is working exactly as I expected. Great work!

@mcarrano Added updates based on your feedback, except the empty state. I cannot get to the full page demo via the link - it returns a 404 page. Is there another place I can look for this? I don't think we have an empty state demo for the table in react yet.

Looks like there is a website issue preventing you from seeing the empty state demo @kmcfaul .

Actually, here is a screen shot of what the empty state should look like @kmcfaul:

You should be able to access this under core demos/table Look for the Table-empty state demo

Is this demo supposed to be in an experimental category?

@mcarrano Added the empty state, thanks for the screenshot & location!

@nicolethoen @rachael-phillips We don't have a demos section in Experimental, but maybe it should be added?

I think we probably don't want to encourage people to implement the toolbar as seen in the demo if regular breaking changes could still be in the component's future. So the demo should be specifically categorized or consumers sufficiently warned... I thought that it was agreed that the demo would be in an experimental section or held off until the component is no longer experimental

When the other PR gets merged, you'll want to add a 'collapseListedFiltersBreakpoint' to the DataToolbar component to make it more responsive, just a heads up. @mcarrano should this filter be in a toggle group so it can be more responsive? adding this prop will make the 'X filters applied' message appear, but without the toggle group, there wont be a way to see which filters have been applied. I'm honestly not sure what the behavior of this multiple drop down select will be when it is used in a toggle group - it should be explored.

Yeah I agree, I'll try creating a new folder for demos in experimental and see if that gets picked up!

Thanks for the heads up about the PR changes as well.

@mcarrano @nicolethoen
Adding in the toggle creates some issues.

• The biggest issue is that the chip groups aren't located properly in the collapsed state's expanded menu for this use case. They are located after their respective dropdown, and contain spacing even when no chips are present. With 2 of the 3 dropdowns hidden, there are varying white space gaps above and below each filter dropdown. I would expect all chip groups to be inline at the end of all dropdowns.
• The inline styling to prevent the dropdowns from resizing also limit them in the collapsed state menu when its expanded. Should I remove these limits?
• It always appears collapsed despite setting a breakpoint. Might be user error on my part, I'm fiddling with it still.

Talked with Nicole and rebased with the data toolbar changes. The responsive collapse and expansion is working now.

I removed the width styling for now, and scheduled a meeting to go over the chip group locations in the collapsed menu.

As this demo uses an experimental component, we are moving forward with this PR and will increment on it in the next release re: the chip group stacking in the collapsed menu.

And people are okay with an experimental component under Demos?

I believe that was what was decided after a recent standup. @tlabaj Can you confirm?

I wonder if this demo should be under Toolbar demo because in Core it is, and the reason for the demo was to show how the new toolbar works when filtering the table. If it sits under Table demo, I don't know what functionality of the table its demonstrating? Seems like its just demonstrating the filtering. Would be great to hear what @mcarrano thinks

This is tricky @christiemolloy . I discussed this with @kmcfaul and @nicolethoen in a meeting earlier today. Naming of demos is something that needs more thought in general, IMO. Should they be based on the thing being demonstrated (i.e. Toolbar) or the use case being demonstrated (i.e. Filtering). I don't know, but the naming will effect how easy it is for developers to find these. Not sure how the website search indexes these (or does it). Let's leave alone for now, but @rachael-phillips @LHinson I think this is a topic that should be on the radar for more discussion.