Skip to content

Conversation

@kmcfaul
Copy link
Contributor

@kmcfaul kmcfaul commented Nov 13, 2025

What: Closes #11893

Still needs some testing and definitely requires a styling update. Functionally the disabled props will work, but they will not look any different.

@patternfly-build
Copy link
Collaborator

patternfly-build commented Nov 13, 2025

Copy link
Contributor

@thatblindgeye thatblindgeye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some general comments after testing and tinkering in the preview:

  • For the "Single selectable" tree view (where the entire node toggles expansion rather than selection, if it expands), the disabled state isn't immediately announced. I have to 1) go into the tree view, 2) navigate to the tree item, then 3) go further into that tree item (rather than navigating forward/backward) to get to the button inside, which then announes a disabled state. I'm not sure if users would typically do that.
    • For this, what we could do instead of putting disabled on the button, put aria-disabled="true" on the role="treeitem" element
  • For the separate expand/select buttons from "With separate selection and expansion" example, sort of the same issue as above, except the other issue is that the navigation as noted above is basically required with VO for this implementation. I have to go deeper into a tree item in order to know that there are separate expand/collapse and select buttons (which, idk if that's really a great implementation in the first place). Via plain keyboard it works okay as I can go to the separate expand and select buttons, but for VO it's not as straight forward.

I'm not sure what the best approach would be off the top of my head. For the more basic tree view, applying aria-disabeld to the treeitem would be better than putting disabled on the button. For an implementation where expansion and selection are separate actions within a tree item, that may not work, but at the same time it may not be clear that you have to go further into the tree item for additional/separate actions. I might want to say that allowing this mixed state of "one action is disabled, but not the other" isn't advisable - either the entire tree item is disabled or it isn't. That would mean we could just apply aria-disabled to the treeitem and have it exposed as disabled far easier for users.

Comment on lines +38 to +41
/** Flag indicating if the tree view item is disabled. */
isDisabled?: boolean;
/** Flag indicating if the tree view item toggle is disabled. */
isToggleDisabled?: boolean;
Copy link
Contributor

@thatblindgeye thatblindgeye Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we want isDisabled to disable everything (selection and expansion)? Might just be me, just that's what I expected to happen when passing isDisabled, but that only disabled selection for the example where the selection is a separate action; for the default behavior isDisabled works as expected since there's only one button.

Or would we want to have isToggleDisabled and isSelectionDisabled props instead?

@thatblindgeye
Copy link
Contributor

TLDR of this comment: how we currently have our TreeView component setup, the way this PR goes about disabling stuff might be one of the better ways to handle it.

The longer story:

So I tried testing out with NVDA a bit as well and looking a little more, and at least based on whatever settings I have set in both NVDA and VO, the interaction experiences are fairly different. In NVDA I can use arrow keys alone to navigate the tree view items, i.e. arrow keys will essentially focus the button elements within each tree item (whether it be the entire button that expands, or separately the expand button and selectable button).

VoiceOver from what I can tell I need to use VO keyboard shortcuts to navigate since it's being announced as a table, in which case my previous comment of it being difficult to tell that there are separate expand/select actions still seems to stand (and in that case, the disabled state may not get announced since VO focus i on the tree item, not the button element within the tree item.

Though NVDA can get a bit complicated as well when navigating via their shortcuts; if I use NVDA navigation within the treeview and try to move browser/system focus to the treeitem that has NVDA focus, I still have to press the Tab key to place focus on the actual button element to interact properly. I can't be sure if this is how a user would actually interact/navigate in a treeview, though.

As an addendum to one of my previous comments, making the li[role="treeitem"] element aria-disabled="true", the button elements within that treeitem element inherit that state it looks like (both in VO and NVDA). That could work most of the time, but maybe not always.

FWIW, WCAG has this navigation tree view example that has a separate selection/expansion sort of implementation. Essentially, the caret for expansion isn't in a semantic element, it looks to rely solely on mouse clicking the icon to expand it without selection while keyboard users need to use left/right arrow keys to expand/collapse. Also their expected keyboard interactions can e found on their tree view pattern page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tree View - allow for more control of underlying Tree View List Items

3 participants