feat(elements): add broadcast channel to sync icons with wc #14498
Conversation
MayaKirova
force-pushed
the
mkirova/poc-icon-service-sync
branch
from
ada18c9
to
e7b055f
Compare
projects/igniteui-angular/src/lib/icon/icon.broadcast.service.ts
Outdated
Show resolved
Hide resolved
MayaKirova
force-pushed
the
mkirova/poc-icon-service-sync
branch
from
fb98273
to
cf75616
Compare
| ], | ||
| // bootstrap: [] | ||
| }) | ||
| export class AppModule { | ||
|
|
||
| constructor(private injector: Injector) {} | ||
| constructor(private injector: Injector, private _iconBroadcast: IgxIconBroadcastService) {} |
There was a problem hiding this comment.
Okay, kind of better we're keeping this in Elements for now.
A bit awkward in terms of usability as it stands having to inject just to trigger the functionality. So it either doesn't need to go through the DI system or perhaps we should consider some API on it like start & stop basically - both to make sense as usage and to have a cleanup mechanism.
Alternatively, since this isn't root-provided it should be able to take advantage of ngOnDestroy for cleanup.
Both of those things technically don't matter much for Elements, since this module will never destroy until the entre app does, but still should be noted.
|
|
||
| if (message.actionType === ActionType.SyncState || | ||
| message.actionType === ActionType.UpdateIconReference) { | ||
| this.updateRefsFromCollection(message.references); |
There was a problem hiding this comment.
Assuming it's the responsibility of the other side to only emit the icons that do need changing for UpdateIconReference?
There was a problem hiding this comment.
Yes. The other side sends the state of the collections and this side updates its own state.
There was a problem hiding this comment.
Hehe, then it's not a "yes" answer :) The question was if "the other side to only emits the icons that do need changing".
If the other side sends the entire state of the collections, every API for change on that side will also cause the Angular side to re-register all previous icons for example, which is at least redundant if it doesn't affect performance much.
There was a problem hiding this comment.
Nvm, checked the implementation, the other side creates a fresh collection for each change, so it is sending just the modified refs. No need to break-check me like that 😄
dkamburov
added
the
✅ status: verified
damyanpetev
changed the title
…en angular and wc.
Goes with: IgniteUI/igniteui-webcomponents#1295
Closes #
Additional information (check all that apply):
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)