chore(Dropdown): updated deprecated usage to new dropdown

[thatblindgeye]

What: Closes #8839

Pages from the surge that were updated (some with notes, other notes after this list):

• Action list single group
• Breadcrumb with dropdown - uses a recreation of the badge toggle
• Card examples that have a kebab and horizontal card grid demo
• Datalist examples that have a kebab, and the demo
• Datetime picker - the time toggle (with clock icon) no longer has the bottom border since I'm using the "plain" MenuToggle variant to remove the caret icon. Could be a Core update or we could introduce a new MenuToggle variant ("controlled"? basically something that keeps the bottom border but removes the caret icon)
• Dual list selector complex options and actions
• Hint examples
• Input group with dropdown
• Masthead demos
• Menu with drilldown breadcrumb
• Modal with dropdown
• Navigation demos - Horizontal, Horizontal nav with horizontal subnav, Manual nav
• Notification drawer - first 2 examples and both demos; these demos + cypress demos need to be looked into as keyboard handling doesn't work as expected (might be due to NotificationDrawerListItem's internal onKeyDown firing)
• Page demos
• Primary detail pattern - Primary-detail card view, Primary-detail data list in card
• Tabs and tables demo
• Toolbar with filters, Toolbar stacked, and Toolbar demos
• Treeview with action items - cannot currently navigate between kebab menu items via keyboard. This may be due to both TreeView and the new Dropdown's internal Menu components both using KeyboardHandler
• Alert group demos
• Card view pattern - see note below

Other notes:

• Demos that use selectable cards + action kebabs (Card view, primary details, etc): clicking the kebab causes the cards' selection to occur
• Looks like the wrapper div that is rendered around Dropdown's Popper prevents MenuToggle from being full height when isFullHeight is passed (we may not need that wrapper anymore since moving forward we're defaulting to an "inline" appendTo, which that wrapper div was just to give the Popper something to append to)
• The new Dropdown inside of PageHeader components (non-Masthead) doesn't have the correct styling

Original comment regarding badge toggle

Wanted to put a draft up to get some opinion on the Menu Toggle with badge implementation (seen in the Breadcrumb with dropdown example here).

With the deprecated Dropdown, there was a separate BadgeToggle sub-component that was used, but MenuToggle doesn't have that. What I did for now in this PR is just pass Badge component as a child to MenuToggle (rather than using the badge prop), and set it to a plainText variant. I figured right now that would be the simplest way to show how a consumer could use this new composable implementation.

Alternatively we could maybe implement a new prop in MenuToggle that would create a "BadgeToggle", but one possible issue is whether that would get confusing since we already have an icon and badge prop on MenuToggle. Some ideas:

• Introduce a new "badge" variant, along with "plain", "default", etc. that would set the markup and styling similar to how BadgeToggle did
• Add a new prop (isBadgeToggle?) that would probably do the same as above
• Show how to use the menu classes to create a custom badge toggle, basically leave it up to the consumer (probably not great since it'd involve hardcoding classes in?)

There may need some Core followup as I did notice the breadcrumb items with a dropdown weren't centered right. Changing the .pf-c-breadcrumb__item "align-items" property to center instead of baseline may help, but I haven't looked into whether that would cause any other issues.

Additional issues:

[patternfly-build]

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

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

[mcarrano]

@mceledonia @mmenestr @andrew-ronaldson would like to get your opinion on this one. See the question posed by @thatblindgeye in the description above. The gist of this is that by deprecating the old dropdown, we loose the badge variant that allows us to make a badge into a toggle. We could add that to Menu Toggle, but there's a question how useful this is. This was a feaure that was requested for condensing a long breadcrumb control. Not aware that it's used anyplace else. Here are two links where you can compare the existing vs. new implementation:

Existing: https://www.patternfly.org/v4/components/breadcrumb#with-dropdown

Updated: https://patternfly-react-pr-8868.surge.sh/components/breadcrumb#with-dropdown

Note the changed positioning of the caret. I'm OK with the updated implementation but also wanted to get your opinion. Don't recall where this is being used, but if you are aware, I can also check with that team as they may need to do some rework to update to PatternFly 5.

[thatblindgeye]

@mcarrano Nicole pointed me towards a demo that has a badge toggle using MenuToggle similar to the old Dropdown BadgeToggle: composable dropdown variants

So I can actually implement that in this update, unless another approach is agreed on. From what I have seen so far, two areas where we're using a badge toggle like this in examples is the Breadcrumb example and the Menu Drilldown with breadcrumb example.

[mcarrano]

Sounds good @thatblindgeye

[kmcfaul]

• Looks like the wrapper div that is rendered around Dropdown's Popper [...]

You should be able to remove the wrapping div if you pass triggerRef and popperRef to the Popper.

For the TreeView action menu, I thought the keyboard handler should prevent propagation up to the outer handler (so if the focus is in the Dropdown that handler should run and not the TreeView) but maybe that needs some tweaking.

[mcarrano]

@thatblindgeye Glad you were able to work out the issue with the badge style dropdown. But I am seeing a spacing difference between this and the current implementation:

Current:

This PR:

Can that be fixed?

[thatblindgeye]

@mcarrano There's a couple of ways we could go about that I think:

1. Just add some whitespace in the two examples that use this badge toggle. It looks like that's all that is supplying the spacing in the custom menus example. Would be quick and easy, leave it up to the consumer to decide how they want to style a customized badge toggle
2. Use our Icon component to wrap the CaretIcon now, then make an update in Core to supply a padding or margin left to pf-c-icon when it's inside of pf-c-badge.

Here's what option 1 looks like with just a single space:

Here's what option 2 looks like (8px left padding on .pf-c-badge > .pf-c-icon:

[mcarrano]

@thatblindgeye Took some time this morning to review other places affected by this change. Here are additional problems I found...

In all full page demos, the user menu is not rendering correctly. See below.

Should look like this:

I'm not entirely comfortable with effect of this on the Date & Time picker as it now feels like the time widget is no longer a part of the control.

I wonder if we'd be better off adding back the border even if it means it means we need to keep the caret.

@mmenestr @andrew-ronaldson what do you think?

[thatblindgeye]

@mcarrano I should be able to resolve the user menu issue based on Katie's comment above.

Will wait for Andrew and/or Margot's input on the timepicker (can also make updates to the badge toggle depending what you think of my latest comment above)

[mcarrano]

@thatblindgeye regarding fixes to the badge dropdown, I'd be OK with either approach. Although option 2 is perhaps a more robust way to do this? Maybe see what others think.

[mmenestr]

@mcarrano @thatblindgeye I agree it's a bit weird that the calendar icon isn't floating, but the time picker icon is. I would expect them both to be treated the same. I don't think having a dropdown styling with caret is much better, but I feel its probably a bit better than no border at all.

This is making me wonder why we created a combined date and time picker, without also creating a combined time + calendar menu to go with it... I feel like in cases where date pickers and time pickers are both needed, it would be better to use the original components side by side rather than this hybrid version which seems a bit odd. But that's a whole other issue.

[thatblindgeye]

@mcarrano Made some updates to help resolve the spacing issue on the badge toggle as well as the dropdown used in full page demos for the username (should now be full height as it was before).

@mmenestr For the Timepicker, I removed the variant so that it matches the second screenshot Matt shared for now. Should we open a followup issue to resolve how we ultimately want that timepicker toggle to look?

[mcarrano]

I took at look at your changes @thatblindgeye . The breadcrumb update looks good. For the Date & Time picker, can we fix this so that the clock icon does not get cut off?

@mmenestr agree about the Date & Time demo. Would you mind opening an issue to reconsider that? It's definitely beyond the scope of this PR.

[thatblindgeye]

@mcarrano For the timepicker icon, it looks like it's being cut off in Firefox due to the div element in the example having a width of only 300px, so it's trying to hide the overflow with an ellipsis (I guess it doesn't work with only an icon; if you add text before the icon the menu toggle has the ellipsis like normal).

Moving that div element to only wrap the text input works, or I can pass the clock icon to the icon prop of MenuToggle which results in a little more spacing between the icon and toggle caret:

If the spacing isn't too bad I'll push that change, since it may make more sense to pass only an icon to the icon prop.

[srambach]

This one looks good with the follow up issues.