chore: update breadcrumb dropdown item arrow to work in RTL

[mcoker]

fixes #9587

This may not be a great way to do this, but these are basically drillup items in the breadcrumb menus.

[patternfly-build]

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

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

[thatblindgeye]

One issue with using the direction prop for the icon is that it's also going to set some other code internal to MenuItem that isn't needed:

patternfly-react/packages/react-core/src/components/Menu/MenuItem.tsx

Lines 249 to 265 in 1d53917

	let drill: (event: React.KeyboardEvent | React.MouseEvent) => void;
	if (direction) {
	if (direction === 'down') {
	drill = (event) =>
	onDrillIn &&
	onDrillIn(
	event,
	menuId,
	typeof drilldownMenu === 'function'
	? (drilldownMenu() as any).props.id
	: (drilldownMenu as React.ReactElement).props.id,
	itemId
	);
	} else {
	drill = (event) => onDrillOut && onDrillOut(event, parentMenu, itemId);
	}
	}

When it's used in Menu, it's also used for a drilldown type of menu, where the item leads to another sub-menu or a parent menu. For these breadcrumbs, it's more just selecting an item within a list that has been truncated to save space, rather than leading to a sub or parent menu.

This may be a case to use the modifier class to mirror the icon in the examples.

[thatblindgeye]

If we wanted to utilize the direction prop to render the icon correctly, we could update the logic inside of Menu around the code lines I linked above so that we're only reassigning the drill variable if direction && (onDrillIn || onDrillOut).

Overall I dont think I feel too strongly if we want to get this in, though. Unless a user is also passing either of those onDrill callbacks in it may not ultimately matter.

[mcoker]

@thatblindgeye we could close this PR (and the core PR) and leave the issues open, and we can work on it after the release or in a breaking change release if that's needed. WDYT? On the core side, I think it's just going to be updating the examples to wrap the icon in the menu__item-toggle-icon element.

[thatblindgeye]

@mcoker Unless anyone else feels it's urgent to fix I'd be fine with that. As it is it could be considered more the example issue rather than an issue with the component.

[wise-king-sullyman]

That sounds like a great plan to me as well.

[tlabaj]

Did we decide we were nit going to make any changes to fix example code to work with RTL since this would be consumer provided? Maybe this belongs in handbook.

[mcoker]

@tlabaj great question 😅. We have updated a few examples, most cases from what I recall are with code that really should be built into our component. For random icons and things included as filler in components, I believe we've just left all of those alone.

This PR is an example of something a menu item should support. Another example is the expand all icon used in a <ToolbarItem variation="expand-all">.

There are issues for both of these to update our components to support making RTL work natively

• The issue this PR addresses - Menu/MenuItem - should support drill-[up/out] icon/layout in menu list #9653
• Toolbar expand-all icon ToolbarItem - expand-all variant should include expand all icon #9641

[patternfly-build]

Your changes have been released in:

• @patternfly/react-code-editor@5.1.1-prerelease.16
• @patternfly/react-core@5.1.1-prerelease.16
• @patternfly/react-docs@6.1.1-prerelease.17
• demo-app-ts@5.1.1-prerelease.15
• @patternfly/react-table@5.1.1-prerelease.16

Thanks for your contribution! 🎉