IconButton is broken with icons that are not supported by fabric-core

[chnldw]

Environment Information

• office-ui-fabric-react 7@latest
  Fabric-Core css @10.1.0
• Chrome@latest, Windows 10

IconButton is broken with icon that is not supported by Fabric Core.
Seems one additional ms-Icon class is added.
Is there any workaround for this?
https://codepen.io/chnldw/pen/jONpKGe?editors=1010

Repro:

1. include fabric-core css.
2. Create an IconButton with PageList Icon.

Expected:
PageList icon rendered correctly.

Actual:
Icon is not rendered. It was working on previous version. around version 7.11

Priorities and help requested:

Are you willing to submit a PR to fix? (No)

Requested priority: (High)

Products/sites affected: (if applicable)

[aneeshack4]

@chnldw So this seems to work if <link rel="stylesheet" href="https://static2.sharepointonline.com/files/fabric/office-ui-fabric-core/10.1.0/css/fabric.min.css"> is removed. This actually works also if you change the version from 10.1.0 to 10.2.0 - can you just update the version of the fabric core css you're using?

[chnldw]

@aneeshack4 thanks for the comments. we do need fabric-core.css in our project. The latest version I believe it's 10.1.0. Change it to 10.2.0 basically the same as remove it as it's a 404.

[vch-7]

@chnldw @aneeshack4
This seems to happen after the addition of the Fast icon variants in v7.25.0, where the IconButton and other components that accept an iconProps prop like DetailsList now render the FontIcon or ImageIcon component by default instead of the Icon component.
These components have an ms-Icon class name that is meant to be used for styling non-themeable Icon components.
However, the default styles for the ms-Icon class seem to be served by Fabric-core which applies the FabricMDL2Icons as the font-family style for the icon element and it seems like the icons that are not supported/provided by Fabric-core do not belong to the FabricMDL2Icons font-family.

A work around(ugly) for this would be to provide a styles object to the iconProps of the IconButton component. For components like the DetailsList you can set the useFastIcons prop to false. This will render the old Icon component instead.

Possible fix:
Use a different class name string(other than ms-Icon) as the MS_ICON class value for styling non-themeable Icon components?

[aneeshack4]

Thanks @varunch7 for the possible workaround & fix. I'm still waiting on hearing back from the author of this code - there's a chance we're not supporting this exact same icon set. Will update y'all when I hear back from them. Meanwhile @chnldw have you tried what @varunch7 has suggested?

[msft-github-bot]

This issue has been automatically marked as stale because it has marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fabric React!

[vch-7]

@aneeshack4 Just checking in to see if you have any updates on this

[aneeshack4]

@varunch7 I spoke with @Jahnp offline - he has some high pri stuff on his plate at the moment but he's going to look at this after.

[msft-github-bot]

This issue has been automatically marked as stale because it has marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fabric React!

[msft-github-bot]

This issue has been automatically marked as stale because it has marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fabric React!

[vch-7]

@aneeshack4 @micahgodbolt Is there any way we can change the tags in a way that the bot does not close this issue for being stale until @Jahnp gets to have a look at this?

[aneeshack4]

@varunch7 Your comment removed the tag so it's no longer stale :) Pinging @Jahnp as a gentle reminder

[Jahnp]

Apologies for the delay folks. While not the ideal fix (you should expect to be able to use Fabric Core + Fabric React and use icons interchangeably between them), this has been temporarily mitigated with OfficeDev/office-ui-fabric-core#1195, which adds ~350 icons to Fabric Core and brings it up to parity with Fabric React.

That PR was incorporated in Fabric Core@11.0.0 due to the removal of 4 icons we no longer have permission to distribute. See this forked version of @chnldw 's codepen for an example: https://codepen.io/jahnp/pen/oNNzGPz?editors=1010

Again, this doesn't solve the bigger underlying issue of Fabric Core's ms-Icon definitions stomping on Fabric React's, but should unblock you in the short term. Let me know if this addresses your concerns, @chnldw and @varunch7 . Thanks again for reporting the issue!

[msft-github-bot]

This issue has been automatically marked as stale because it has marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fabric React!

[vch-7]

Thanks @Jahnp for the update! The icon parity should mitigate this issue. But right now, we get the Fabric core styles by importing Fabric.min.css using
import 'office-ui-fabric-react/dist/css/fabric.min.css'
This still points to v10.1.0 since the version bump to v11 does not seem to have been checked into master yet for it to be consumed.
There do not seem to be any issues tracking this as of yet.
I was wondering if there is a timeline as to when fabric.min.css would point to v11?
Thanks!

[msft-github-bot]

This issue has been automatically marked as stale because it has marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fabric React!

[KevinTCoughlin]

I vote we keep this issue open. It is a significant regression that partners have to workaround via CSS scoping; at least that is how SPO dealt with it.

Exit criteria should at least be updated documentation, if not a fix for the issue. Note that the existing "Getting started" documentation link at https://developer.microsoft.com/en-us/fabric#/styles/web/icons 404s.

Again, this doesn't solve the bigger underlying issue of Fabric Core's ms-Icon definitions stomping on Fabric React's

Can the above be fixed? Or at least documented publicly. Up until recently both libraries worked side-by-side and existing documentation alludes to that being a supported scenario.

cc: @patmill @aditima @qianghuang94

Related: SharePoint/sp-dev-docs#4911

[ecraig12345]

I reopened it just now because I think it got automatically closed by mistake (the "needs author feedback" label isn't handled properly if someone other than the original author responds).

[aneeshack4]

That sounds reasonable @KevinTCoughlin. @Jahnp What do you think about these updated changes to the documentation? Looks like @maxwellred might be make these changes in #10987 per @ecraig12345's comments

[ecraig12345]

I asked @maxwellred to fix the broken link as part of his fabric core upgrade PR #10987. The larger documentation update should probably be done separately.

[maxwellred]

Just a heads up, @chnldw you can use Fabric core 11 as you can see in this codepen https://codepen.io/CodeMax/pen/eYmNXem?editors=1010 and @varunch7 that css min file now points to Fabric core 11 as well.

[ecraig12345]

A temporary workaround for this issue (which I'd forgotten about until looking at the code just now) is to include styles: {} in iconProps -- for example iconProps={{ iconName: 'PageList', styles: {} }}. https://codepen.io/ecraig12345/pen/eYmgwNJ?editors=0010

The presence of styles causes the button to fall back to rendering the old Icon component which doesn't add ms-Icon (instead of the new optimized FontIcon which does add the class).

If anyone is having an issue with DetailsList not showing icons, a temporary workaround there is to set useFastIcons={false} in the props to use the old Icon.

[ecraig12345]

I talked with @Jahnp about this today and came up with a potential fix--see #11524.

[msft-github-bot]

🎉This issue was addressed in #11524, which has now been successfully released as @uifabric/styling@v7.8.0.:tada:

Handy links:

• Release Notes
• Official NPM Release

[msft-github-bot]

🎉This issue was addressed in #11524, which has now been successfully released as office-ui-fabric-react@v7.76.2.:tada:

Handy links:

• Release Notes
• Official NPM Release
• Test the release with Developer Demo Site