patternfly / patternfly/patternfly-react
Menu/MenuItem - should support drill-[up/out] icon/layout in menu list
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 862
- Forks
- 392
- Avg merge
- 4d 8h
- Merged PRs (30d)
- 9
Description
re: https://github.com/patternfly/patternfly-react/issues/9587 and this issue to make the same update in core https://github.com/patternfly/patternfly/issues/5950. Not sure if this is a breaking change or not, probably depends on how we go about implementing it.
In the drilldown with breadcrumb menu examples, in the breadcrumb dropdown, we have items that present (and seem to function) as drill-[up/out] items with a back arrow at the start of the item, and clicking on the item takes you up a level in the breadcrumb hierarchy. However, our component doesn't support this layout - our examples are using a user-supplied icon passed as the icon prop - https://github.com/patternfly/patternfly-react/blob/e27aac2f35e0a8bb16fa5397eb5ef841b3ecc578/packages/react-core/src/components/Menu/examples/MenuWithDrilldownBreadcrumbs.tsx#L130-L138
Some issues with this are:
- It doesn't work by default in RTL since we can't predictably target and mirror/flip that menu item icon for the user since that could be a non-directional icon or they may have opted for some other type of directional icon there that shouldn't be flipped.
- Since it's a user supplied icon, there is the possibility for inconsistency
- I could be wrong, but I don't know if a user could pass an actual icon to any of those items since the prop is being used for the back arrow.
We have a direction="up" prop that puts the correct icon container and icon on a drill-up/out icon and works in RTL, but that is specific to the top/first item in a drilled-in drilldown menu. The prop applies other logic to that item that isn't appropriate for use in the breadcrumb items, so we can't use that prop there. https://github.com/patternfly/patternfly-react/blob/e27aac2f35e0a8bb16fa5397eb5ef841b3ecc578/packages/react-core/src/components/Menu/MenuItem.tsx#L366-L370
Ideally we could do something like update the direction prop so it could be used in these breadcrumb menu items or maybe introduce some new kind of prop for an item that indicates clicking it does some kind of "back" action and it just borrows the way direction="up" includes the icon in the item.
Jira Issue: PF-116
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with packages/react-core/src/components/Menu/MenuItem.tsx, especially the existing direction="up" handling, and the MenuWithDrilldownBreadcrumbs.tsx example linked in the issue. Compare the current breadcrumb dropdown behavior with the core issue and RTL expectations. Done means the breadcrumb drill-up/out items have a consistent, direction-aware layout without relying on a user-supplied back icon.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react, typescript
- Domain
- frontend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100