OfficeDev / microsoft-teams-ui-component-library

Component library enhanced experiences styled for use in Microsoft Teams custom applications.
MIT License
127 stars 30 forks source link

Allow ReactElements to be used as icons #63

Closed dvdzkwsk closed 3 years ago

dvdzkwsk commented 3 years ago

Currently we have a limited, hardcoded list of icons. As seen in https://github.com/OfficeDev/microsoft-teams-ui-component-library/issues/62, it is extremely likely that a user will want to use an icon that isn't in this list. The most straightforward solution is to allow a React element to be passed in place of a string value, which allows users to provide any icon they choose (ideally one from @fluentui/react-icons-northstar).

One drawback of allowing icons to be React elements is that the JSON configuration is no longer serializable. Honestly, I think that's reasonable since on top of being idiomatic React, it's a userland decision and if we needed to serialize it back we could (read: should) sanitize it anyways. Still, worth calling out. The other drawback is that it allows any component to be used as an icon. If we're hoping to inspire consistency across the Teams platform then this works against that. It might be wise for this to be documented primarily as an escape hatch and that developers should stick to Teams icons wherever possible.

A different solution would be to offer an API such as:

const registry = new Map()
export const registerIcon = (name: string, component: React.ComponentType) => {
  registry.set(name, component)
}
const Icon = ({ icon }: IIconProps) => {
  const Icon = registry.get(icon)
  return Icon ? <Icon /> : null
}
registerIcon("MyCustomIcon", MyCustomIcon)

I'm personally not a fan of this as the primary API because it means all used icons must be registered before they can be used, which would increase bundle size if done upfront or possibly buggy behavior if done as-needed. Users would be able to lazy-load the icon to circumvent the bundle size issue, but at that point the API simply becomes unergonomic.

That said, it may offer a reasonable addition to this change if we're keen on representing component API's strictly as serializable JSON.

Lastly, a third solution would be to enumerate all "approved" icons ahead of time and manage the lazy-loading ourselves. I definitely do not think this is a worthwhile pursuit, but wanted to mention it for completeness and to highlight the drawbacks of supporting, and restricting users to, a large catalog of icons. This would become even more tricky since we treat the Fluent icons package as a peer dependency. In short: there be dragons.

bxav commented 3 years ago

I don't think we should begin to allow React components as props to the HVCs without also revisiting the primary goals underpinning this project.

If we want to keep those goals the same, as you put it…

to inspire consistency across the Teams platform

…I think the best approach for the long-term would be to move the icons library to the direct dependencies and go with your third suggestion, to enumerate them and lazy load them when needed.

The Icons are just an example, but I understand your point, on a private library this make sense. But to be honest as an external consumer if I can't extend the Icon, List, or the Board components (for example extending the BoardItemDialog) I will have to fork this library and extend it for my own use. But that means I will lose all the great work you are doing.

thure commented 3 years ago

as an external consumer if I can't extend the Icon, List, or the Board components … I will have to fork this library

The main goal of this library is to provide experiences that are true to Microsoft Teams’ UI Kit’s design patterns, and avoid experiences that aren’t accessible or feel alien to the platform.

@bxav, If the ways you want to extend this library are coherent with the design system’s patterns, I think a PR with those extensions would be accepted. We may also be able to prioritize designs for the extensions you want to see if you’re able to share those use-cases with us. If it’s about icons, you could share that here, or if the scope of those use-cases is different you could create a new issue to discuss them.

dvdzkwsk commented 3 years ago

Closed by https://github.com/OfficeDev/microsoft-teams-ui-component-library/pull/69