Dmceachernmsft/ChatButtonCustomIcons#1524
Conversation
dmceachernmsft
requested review from
JamesBurnside,
PorterNan,
alkwa-msft,
anjulgarg,
carocao-msft,
edwardlee-msft,
mgamis-msft,
prabhjot-msft and
prprabhu-ms
as code owners
| ControlBarChatButtonInactive: <Chat20Regular />, | ||
| /* @conditional-compile-remove-from(stable): meeting/calling-composite */ | ||
| ControlBarPeopleButton: <People20Regular />, | ||
| /* @conditional-compile-remove-from(stable): meeting/calling-composite */ |
There was a problem hiding this comment.
Further down in this file, this should have a CallWithChatCompositeIcons interface that 'picks' the ControlBarChatButtonActive, ControlBarChatButtonInactive etc.
And just noticing, CallCompositeIcons should not have
| /* @conditional-compile-remove-from(stable) Chat_Notification_Icon */ 'ControlBarButtonBadgeIcon' below
There was a problem hiding this comment.
(would be better as a separate PR)
There was a problem hiding this comment.
In fact, I needa do some API changes after ARB review, I'll make these changes when this PR goes in
There was a problem hiding this comment.
this was something I was wondering about when working on this! those types at the bottom weren't needed to implement this. is that for the base composite to know what are valid icons to override?
There was a problem hiding this comment.
It's for contoso developers to easily have a list of icons they can override, though it's a shame we don't have any typeguarding when using icons to say - "hey! this icon isn't part of the CallWithChatCompoisteIcons interface! I'm, gonna fail your build"
We probably could - we would have to define our own Chat/Call/CallWithChatCompositeIcon that wraps the fluent icon and only allows a subset of strings in the iconName that exist in the xCompositeIcon interface
There was a problem hiding this comment.
cool! I thought it might be something like this when intellisense didn't see the other icons right away!
There was a problem hiding this comment.
@JamesBurnside might be worth doing actually, I like that idea.
<CallIcon iconName="something" />
<ChatIcon iconName="something" />
<CallWithChatIcon iconName="something" />
There was a problem hiding this comment.
Actually doing it now! Along the lines of this:
….com/Azure/communication-ui-library into dmceachernmsft/ChatButtonCustomIcons
….com/Azure/communication-ui-library into dmceachernmsft/ChatButtonCustomIcons
3 participants
What
Move Icon registration for chat and people buttons in callwithcomposite
Why
This allows for customization of icons in composite through existing API for customizing icons
https://skype.visualstudio.com/SPOOL/_workitems/edit/2769959
https://skype.visualstudio.com/SPOOL/_workitems/edit/2769961
How Tested
Ran locally on my machine passing in custom Icons