Skip to content

Conversation

ok7sai
Copy link
Member

@ok7sai ok7sai commented Aug 28, 2025

WIP: fixing tests

--
This change makes composite patterns work in a natural data-down flow with no more circular dependencies.

Problems with current implementation

  • Toolbar and Radio Group are strongly coupled, each patterns contain complex conditional logics when each other pattern exists to handle event listener conflicts.
  • This also creates circular dependencies, and there are many ...Like interfaces created as a workaround.
  • Not future proof. The above two problems will grow exponentially if a pattern needs to support multiple composite patterns.

With this refactoring

  • Event handlers are pulled out from patterns, and instead patterns provide actions of what they are capable to do.
  • Radio Group supports two sets of actions - as regular radio group and as a sub group in a toolbar.
  • Toolbar supports a new child pattern - Widget Group, which will treat everything inside as a blackbox. What it does is passing down the interactions from Toolbar, and whatever in the blackbox should handle those interactions.
  • Now the control-flow becomes clear
    • On the top level, Toolbar-Interaction converts keyboard/pointer events into available Toolbar actions
    • Widget and Widget Groups are both list items. If the current activeItem is a Widget Group, then keep passing down the Toolbar action.
    • Whatever inside the Widget Group(in this case it's Radio Group) takes the instructions from Widget Group and handle the interactions by themselves.

As a result

  • This solves event handler conflicts in a composite pattern as patterns no longer need to bind their event handlers to a DOM.
  • Composite pattern parent no longer need to know what children patterns they are controlling. They only need to pass down instructions, and children patterns should implement their interop layer to handle those instructions, and return necessary information to the parent. Very typical Data Down Action Up style.
  • This approach also enables the potential of supporting non-Angular Aria library components, for example, as long as Material Radio Group can act on instructions sent from Widget Group, then it can be used within the Angular Aria Toolbar, without having to tweak anything inside the Toolbar. It can't be done with the current implementation.

@ok7sai ok7sai requested a review from wagnermaciel August 28, 2025 17:01
@ok7sai ok7sai added the dev-app preview When applied, previews of the dev-app are deployed to Firebase label Aug 28, 2025
Copy link

Deployed dev-app for 2bc2c36 to: https://ng-dev-previews-comp--pr-angular-components-31802-dev-ze9ur2vw.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

@wagnermaciel
Copy link
Contributor

I'm not convinced separating the toolbar interactions from the toolbar & radio patterns is a net benefit. Can you explain the intended goal of this refactor?

@ok7sai
Copy link
Member Author

ok7sai commented Aug 29, 2025

I updated the description with details. The title is confusing for sure :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev-app preview When applied, previews of the dev-app are deployed to Firebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants