fix(select): disable focus trap on checkbox select with no children

[boaz0]

What:

It seems like Focus Trap throws exception if no children to ref. In order to avoid this error, render the component with no Focus Trap when no children is passed.

closes #2618

//cc @SpyTec

[patternfly-build]

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

[jenny-s51]

Hi @boaz0, your changes look great! When I set the select dropdown to have no children (delete the options object from CheckboxSelectInput), the focus trap error no longer appears. However, if I try to expand it looks like this.

I'm wondering what should be the default behavior of the select dropdown if there are no children. Though it's also possible that I'm not testing it like you did... how did you test your changes?

[SpyTec]

Hi @jenny-s51, I would say that issue is out of scope for this PR. The behavior exists on all select components so it's probably better suited for patternfly-next

[jenny-s51]

@SpyTec Makes sense to me -- thank you for clarifying Eric!

[jenny-s51]

LGTM 👍

[boaz0]

@jenny-s51 sorry for the late response. I actually thought an empty div is an expected behavior. Though, I agree with @SpyTec when he said we should ask the pf4 core team what they think about it.

//cc @mcoker @mcarrano - can you take a look at this. What should the expanded select item look like when there are no children/options?

[mcarrano]

Can you give me an example use case of how/where this would show up?

[jenny-s51]

Yes @mcarrano please see patternfly/patternfly-next#2132 (comment) for @SpyTec 's response to your question

[mceledonia]

Chiming in here (I think a typo led me here) but I'd think just a basic text-only empty state, maybe secondary text color body style "No options available" or some similar language would work! Same spacing as a single entry in the list... nothing too fancy needed. Scratch that, I was thinking this was for just a dropdown with no options available vs. none yet loaded.

[jschuler]

This fixes an bug that is already present (throws an exception if no children present), merging this and in the meantime we can explore a loading pattern/skeleton for when there are no children in patternfly/patternfly-next#2132