feat(Icons): update icon svg factory example & added components tests #12675
Conversation
-added components tests
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 49c040e8963a9b242083a782b531ec35845fde5d (build) |
Perf Analysis
All results
Perf Analysis (Fluent)Perf comparison
Perf tests with no regressions
|
packages/office-ui-fabric-react/src/components/Icon/examples/Icon.SvgFactory.Example.tsx
Outdated
Show resolved
Hide resolved
packages/office-ui-fabric-react/src/components/Icon/examples/Icon.SvgFactory.Example.tsx
Outdated
Show resolved
Hide resolved
| }); | ||
|
|
||
| const icons = Object.keys(ReactIcons).reduce((acc: React.FC[], exportName) => { |
There was a problem hiding this comment.
We may want to not load all 2000+ icons here but rather a few key ones, or have a separate page as an icon viewing page.
There was a problem hiding this comment.
Note that this page ends up showing in both the public doc site and the snapshot tests.
There was a problem hiding this comment.
Actually I am showing 100 icons at a time, as I have pagination. I didn’t know what’s the best way of testing all icons with screenshots, so let’s sync on that.
packages/office-ui-fabric-react/src/components/Icon/examples/Icon.SvgFactory.Example.tsx
Outdated
Show resolved
Hide resolved
| expect(ReactIs.isValidElementType(IconComponent)).toEqual(true); | ||
| }); | ||
|
|
||
| test(`${file} matches displayName`, () => { |
There was a problem hiding this comment.
May be good to add a few more conformance tests like it can take a className prop, what about passing along data- attributes, do we support as (I don't think so but can't remember.)
There was a problem hiding this comment.
Will add the data- attributes and className tests, and you are right we do not support as prop.
There was a problem hiding this comment.
Maybe slim down how many icons we show here. This could be merged first and then that could be addressed in the other uber PR.
Would be good to make sure we have a few more test cases covered though. :)
I will approve, let's get this one merged first. I also checked in a change to publish this package, as the fabric package was depending on a private unpublished package which broke some cases. My bad for not thinking about that. But we will look at beachball to prevent these scenarios in the future.
…con.SvgFactory.Example.tsx Co-Authored-By: David Zearing <dzearing@microsoft.com>
I have pagination, I show only 100 icons on page.
Will add the recommended tests, but will add them in
This is scary, how this even happend? Aren't we automatically releasing the packages created? :\ Would be good if you can reference some guide for this if there is. |
|
🎉 Handy links: |
|
🎉 Handy links: |
…microsoft#12675) * -changed svg factory example -added components tests * Change files * -fixed import * -fixed example and updates snapshot * -converted to mergeStyleSets * -addressed comments * Update packages/office-ui-fabric-react/src/components/Icon/examples/Icon.SvgFactory.Example.tsx Co-Authored-By: David Zearing <dzearing@microsoft.com> * -added tests * Change files * -description Co-authored-by: David Zearing <dzearing@microsoft.com>
This PR is continuing the work from #12608
Things done in this PR: