feat: icon provider custom icons#4352
Conversation
|
|
||
| export default function CustomIconsPage() { | ||
| return ( | ||
| <article> |
There was a problem hiding this comment.
nit: You can use SimplePage helper for this page instead. Passing screenshotArea={{}} to it will enable the internal screenshot area for it, too.
| @@ -0,0 +1,8 @@ | |||
| { | |||
| expect(firstIcon).toStrictEqual(expectedSvg); | ||
| expect(secondIcon).toStrictEqual(expectedSvg); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
should we add tests for the following cases:
- Custom icon is defined using provider and is then overridden with a nested provider
- Custom icon override behaviour when passing
{ rocket: undefined }to the nested provider
| expect(icons).toHaveProperty('close'); | ||
| }); | ||
|
|
||
| it('accepts arbitrary string keys', () => { |
There was a problem hiding this comment.
What is this test for, isn't it captured by e.g. the first one?
There was a problem hiding this comment.
Good point, I simplified/reworked this, and added a type assertion test to validate the object keys would return from defineIcons as exact values, are weren't widened to string.
| expect(icons).toHaveProperty('zap'); | ||
| }); | ||
|
|
||
| it('accepts a mix of built-in and custom keys', () => { |
| } | ||
| } | ||
|
|
||
| function setupTest(testFn: (page: CustomIconsPage) => Promise<void>) { |
There was a problem hiding this comment.
I think these tests are not really needed, because:
- The logic can be and is tested by the unit tests
- We will provide an internal screenshot test to capture the actual render output of the page - it will capture the presence of the icons and also the way they render, which can include colour inheritance and size variants, once supported.
bmv437
force-pushed
the
feature/custom-icons
branch
from
cb286be to
8f032c4
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4352 +/- ##
=======================================
Coverage 97.44% 97.44%
=======================================
Files 897 901 +4
Lines 26397 26422 +25
Branches 9527 9523 -4
=======================================
+ Hits 25723 25748 +25
Misses 631 631
Partials 43 43 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3ea9572
2 participants
Description
Adds custom icon support to
IconProvider, allowing consumers to register arbitrary icon names (beyond the built-in set) and use them across all Cloudscape components that accept icon names.Notable changes:
IconRegistry/IconMaptypes — Consumers use TypeScript module augmentation (declare module) to register custom icon names, making them type-safe acrossIcon,Button, and all other icon-accepting components.defineIconshelper — A type-identity function that lets consumers define a custom icon map (defineIcons({ rocket: <RocketSvg /> })).IconProps.Nameunion type is replaced by a sharedIconNametype defined inicon-provider/interfaces.ts.IconProviderProps.Iconsis now a mapped type overIconNameinstead of a hand-maintained interface with ~130 explicit keys. This is now possible due to this update to documenter.InternalIconrenders custom icons from context and warns when a name isn't found. NestedIconProviders inherit custom icons from parent context, andnullvalues correctly fall back to either the built-in default or the inherited custom icon.defineIcons,IconRegistry, andIconMapare exported fromicon-provider/index.tsx.API proposal: oY6SACNg4aOu
How has this been tested?
src/icon-provider/__tests__/define-icons.test.ts): Validates thatdefineIconsreturns the same object reference and accepts built-in, custom, and mixed icon keys.src/icon-provider/__tests__/icon-provider.test.tsx): New test cases for rendering custom icons through context, inheriting across nested providers, null-reset fallback behavior, and console warnings for missing icons.src/icon-provider/__integ__/custom-icons.test.ts): Browser tests verifying custom icon rendering inIconandButton, built-in icon coexistence, nested provider inheritance, and null-reset behavior.pages/icon-provider/custom-icons/): Visual test page exercising all scenarios with a separatetsconfig.jsonto isolate module augmentation.Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.