Skip to content

Conversation

@g-cheishvili
Copy link

This pull request refactors the AccordionGroup and AccordionTrigger classes to improve extensibility and internal access for derived classes. Most private members and methods are now marked as protected, and some initialization logic is moved to a protected property. Additionally, the ACCORDION_GROUP token is now exported in the public API.

Refactoring for Extensibility

  • Changed most private members and methods in AccordionGroup and AccordionTrigger to protected to allow access in subclasses. (src/aria/accordion/accordion-group.ts, src/aria/accordion/accordion-trigger.ts) [1] [2] [3]
  • Moved effect initialization in AccordionGroup from the constructor to a protected readonly _effects property, making it easier for subclasses to extend or override effect logic. (src/aria/accordion/accordion-group.ts) [1] [2]

Public API Update

  • Added ACCORDION_GROUP to the public exports in public-api.ts, making it available for consumers of the library. (src/aria/accordion/public-api.ts)

@pullapprove pullapprove bot requested review from andrewseguin and mmalerba January 1, 2026 10:31
export class AccordionGroup {
/** A reference to the group element. */
private readonly _elementRef = inject(ElementRef);
protected readonly _elementRef = inject(ElementRef);
Copy link
Member

Choose a reason for hiding this comment

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

cc @wagnermaciel. I was under the impression that we intended for these directives to be used through hostDirectives.

@g-cheishvili
Copy link
Author

Actually never mind, the changes that I've made do not achieve what I wanted to achieve. With or without host directive this module does not give the flexibility. It only gives you ability to have contentChildren. meaning I can not have a e.g. custom accordion group component and have custom components for items which contain those trigger and panel in their templates. I must have full template defined in the content children of the accordion group. I tried to override the behavior by overriding the _trigger and _panels in extended class, but the problem with that is that the _triggers and the _panels are once initialized with the contentChildren and when child overrides, onDirty method of the contentChild gets invoked on the node, which is not a queryList kind. So no overriding anything at all. Apparently these aria components are not meant to be composed, they're just that, just components, not building blocks

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.

2 participants