Skip to content

refactor(multiple): implement generic child discovery with SortedCollection#33151

Open
ok7sai wants to merge 1 commit intoangular:mainfrom
ok7sai:refactor/sorted-collection
Open

refactor(multiple): implement generic child discovery with SortedCollection#33151
ok7sai wants to merge 1 commit intoangular:mainfrom
ok7sai:refactor/sorted-collection

Conversation

@ok7sai
Copy link
Copy Markdown
Member

@ok7sai ok7sai commented Apr 27, 2026

Migrated Accordion, Listbox, and Tabs this time.

@ok7sai ok7sai force-pushed the refactor/sorted-collection branch 6 times, most recently from 8f522e8 to fbd2456 Compare April 27, 2026 16:33
@ok7sai ok7sai force-pushed the refactor/sorted-collection branch from fbd2456 to 177661f Compare April 27, 2026 17:03
@ok7sai ok7sai marked this pull request as ready for review April 27, 2026 17:13
@ok7sai ok7sai requested review from crisbeto and tjshiu April 27, 2026 18:05
providers: [{provide: ACCORDION_GROUP, useExisting: AccordionGroup}],
providers: [
{provide: ACCORDION_GROUP, useExisting: AccordionGroup},
{provide: ACCORDION_COLLECTION, useFactory: () => inject(AccordionGroup)._collection},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Couldn't whoever is injecting the ACCORDION_GROUP read the _collection off it directly?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oops yea you are right. It has gone too far. I was thinking of supporting multiple collections like

{provide: COLLECTION_A, useFactory: () => inject(AccordionGroup)._collectionA},
{provide: COLLECTION_B, useFactory: () => inject(AccordionGroup)._collectionB},

but the fact it can just read ._collectionA and ._collectionB.

Let me clean those up.

providers: [{provide: LISTBOX, useExisting: Listbox}],
providers: [
{provide: LISTBOX, useExisting: Listbox},
{provide: LISTBOX_COLLECTION, useFactory: () => inject(Listbox)._collection},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same note here about reading the collection off the listbox.

});
}

ngOnInit() {}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This isn't doing anything.

Comment thread src/aria/tabs/tabs.ts
providers: [{provide: TABS, useExisting: Tabs}],
providers: [
{provide: TABS, useExisting: Tabs},
{provide: TABS_PANEL_COLLECTION, useFactory: () => inject(Tabs)._collection},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also the collection here.

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