Skip to content

Conversation

@wagnermaciel
Copy link
Contributor

  • Create a new Tree behavior and refactor the Tree UI Pattern to use it.

@wagnermaciel wagnermaciel requested a review from ok7sai December 18, 2025 13:52
@pullapprove pullapprove bot requested a review from tjshiu December 18, 2025 13:52
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Dec 18, 2025
Copy link
Member

@ok7sai ok7sai left a comment

Choose a reason for hiding this comment

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

Is this the prework for migrating menu to use the tree behavior?

@wagnermaciel
Copy link
Contributor Author

Is this the prework for migrating menu to use the tree behavior?

@ok7sai Yep 👍🏽

* The list of items to navigate through.
* Defaults to the list of items from the inputs.
*/
items?: T[];
Copy link
Member

Choose a reason for hiding this comment

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

The items override feels a bit weird, because whether an override is provided or not, the moving index is always based on the activeIndex from the original input, and the following case becomes hard to reason out.

  • ListNavigation is initiated with items A, and an active index of A.
  • Navigation happens and a different items B is provided.
  • Some logic internally handles the mismatch between active index of A and the items B.

This last part is implicit and not easy to observe.

This makes me think about maybe ListNavigation behavior should just be a set of utility methods, and each method always take all necessary informations (items, activeIndex, wrap, etc.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think ListNavigation should be a set of utility methods since it would require a lot of args in order to perform its operations. Either way, this is outside the scope of this PR.

This strange behavior can be replicated by changing the inputs.items currently. We could prevent this misuse of the behavior with some strict checks that warn or error if these come up. I think we are over-anticipating future concerns that I'm not convinced we will actually encounter

@wagnermaciel wagnermaciel removed the request for review from tjshiu December 29, 2025 15:40
@wagnermaciel wagnermaciel added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Dec 29, 2025
@wagnermaciel wagnermaciel changed the title feat(aria/tree): create tree behavior refactor(aria/tree): create tree behavior Dec 29, 2025
* Create the new Tree Behavior class.
* Refactor the existing Tree Pattern class to use the new behavior.
@wagnermaciel wagnermaciel merged commit 21650a6 into angular:main Dec 29, 2025
28 of 31 checks passed
@wagnermaciel
Copy link
Contributor Author

This PR was merged into the repository. The changes were merged into the following branches:

wagnermaciel added a commit that referenced this pull request Dec 29, 2025
* Create the new Tree Behavior class.
* Refactor the existing Tree Pattern class to use the new behavior.

(cherry picked from commit 21650a6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker detected: feature PR contains a feature commit target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants