patternfly / patternfly/patternfly-react

ToolbarItem - expand-all variant should include expand all icon

Open
#9,641 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Breaking change :boom: PF Team
Dominant language
TypeScript
Stars
862
Forks
392
Avg merge
4d 8h
Merged PRs (30d)
9

Description

re: https://github.com/patternfly/patternfly-react/pull/9605/files

The expand all icon is currently something the user passes, but for us to mirror it automatically in RTL in the CSS, our component should be updated to include it for the user. This button is primarily for an expandable data-list and table, and the expand toggles in those components supply the icon, so the toolbar probably should, too, and would provide consistency in the icons used. We could probably do it a couple of ways:

  1. Keep everything the way it is now, but update <ToolbarExpandIconWrapper> to include the icon for the user. And rename it to <ToolbarExpandAllIcon>? That aligns with the core class. IMO that should be updated whether we use it as a wrapper or not.
  2. Update <ToolbarItem variant="expand-all"> to bundle the 1) toggle button, 2) <ToolbarExpandIconWrapper>/<ToolbarExpandAllIcon>, and 3) the icon. Chatting with @thatblindgeye, we could also allow tooltip props in case someone wants a tooltip for the button.

As far as I can tell, the data list and table do not allow the user to pass their own icon to the expandable toggle, so I don't know that we need to allow for that in the toolbar, either? If we do, there is a question of whether it should go in <ToolbarExpandIconWrapper>/<ToolbarExpandAllIcon> or not, which will rotate (when expanded) and mirror (in RTL) the icon the user passes.

My vote would be for the second option.


Jira Issue: PF-118

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by inspecting the ToolbarItem expand-all variant and ToolbarExpandIconWrapper implementation, then compare them with the expandable toggles in the data-list and table components. Resolve which proposed API to use, and verify that the resulting icon is consistent in expanded and RTL states.

Written by the indexing model from the issue text.

Assessment

Tech stack
react, typescript
Domain
frontend
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.