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

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

Labels

detected: feature PR contains a feature commit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants