-
Notifications
You must be signed in to change notification settings - Fork 21
feat(Icon Proposal): New Icon component for FBR CDN usage #1519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
857291a
aa5ecaa
a7fd524
5523e57
9fe1b08
e27ed4b
721aef7
dc61e04
e80da30
da6ac80
5d9f8c9
53603df
6e4d0f9
4dc38c5
4cbbcc6
6dc1795
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,14 +13,44 @@ const getCases = () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return cases; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test.each(getCases())('Icons catalog for %s (%s)', async (skin, type) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const page = await openStoryPage({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: 'icons-catalog--catalog', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| device: 'DESKTOP', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| skin: skin as (typeof SKINS)[number], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| args: {light: type === 'light', regular: type === 'regular', filled: type === 'filled', size: 32}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test.each(getCases())( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'Icons catalog for %s (%s)', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async (skin, type) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const page = await openStoryPage({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: 'icons-catalog--catalog', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| device: 'DESKTOP', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| skin: skin as (typeof SKINS)[number], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| args: {light: type === 'light', regular: type === 'regular', filled: type === 'filled', size: 32}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const icons = await page.screenshot({fullPage: true}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(icons).toMatchImageSnapshot(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Mock CDN icon requests to return immediately with a simple SVG | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.setRequestInterception(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| page.on('request', (interceptedRequest) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| interceptedRequest.url().includes('mistica-icons') && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| interceptedRequest.url().endsWith('.svg') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| interceptedRequest | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .respond({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| status: 200, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| contentType: 'image/svg+xml', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| body: '<svg width="24" height="24" viewBox="0 0 24 24"><circle cx="12" cy="12" r="10" fill="currentColor"/></svg>', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .catch(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Ignore errors if request was already handled | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| interceptedRequest.continue().catch(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Ignore errors if request was already handled | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Reload the page to ensure ALL icon requests go through the interceptor | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.reload({waitUntil: 'networkidle2'}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Scroll through the page so that all lazy-loaded icons enter the viewport | |
| await page.evaluate(() => { | |
| window.scrollTo(0, 0); | |
| }); | |
| // Incrementally scroll down by one viewport height at a time until the bottom | |
| // of the document is reached, giving icons time to load at each step. | |
| // This keeps the screenshot deterministic even if icons load only when visible. | |
| // eslint-disable-next-line no-constant-condition | |
| while (true) { | |
| const shouldContinue = await page.evaluate(() => { | |
| const currentScroll = window.scrollY || window.pageYOffset; | |
| const viewportHeight = window.innerHeight; | |
| const scrollHeight = document.documentElement.scrollHeight || document.body.scrollHeight; | |
| // If we're already at (or very near) the bottom, stop scrolling. | |
| if (currentScroll + viewportHeight >= scrollHeight - 5) { | |
| return false; | |
| } | |
| window.scrollBy(0, viewportHeight); | |
| return true; | |
| }); | |
| // Give the browser a moment to render newly visible icons | |
| await page.waitForTimeout(250); | |
| if (!shouldContinue) { | |
| break; | |
| } | |
| } | |
| // Return to the top of the page before taking the full-page screenshot | |
| await page.evaluate(() => { | |
| window.scrollTo(0, 0); | |
| }); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,5 @@ | ||||||||||||||||||
| import * as React from 'react'; | ||||||||||||||||||
| import {Avatar, IconBrainRegular, IconFireRegular, IconStarFilled, ResponsiveLayout, Box} from '..'; | ||||||||||||||||||
| import {Avatar, IconStarFilled, ResponsiveLayout, Box, Icon} from '..'; | ||||||||||||||||||
| import avatarImg from './images/avatar.jpg'; | ||||||||||||||||||
|
|
||||||||||||||||||
| import type {Variant} from '../theme-variant-context'; | ||||||||||||||||||
|
|
@@ -43,6 +43,9 @@ type Args = { | |||||||||||||||||
| border: boolean; | ||||||||||||||||||
| }; | ||||||||||||||||||
|
|
||||||||||||||||||
| const IconFireRegular = () => <Icon name="fire-regular" />; | ||||||||||||||||||
| const IconBrainRegular = () => <Icon name="brain-regular" />; | ||||||||||||||||||
|
Comment on lines
+46
to
+47
|
||||||||||||||||||
| const IconFireRegular = () => <Icon name="fire-regular" />; | |
| const IconBrainRegular = () => <Icon name="brain-regular" />; | |
| const IconFireRegular = (props: React.ComponentProps<typeof Icon>) => ( | |
| <Icon {...props} name="fire-regular" /> | |
| ); | |
| const IconBrainRegular = (props: React.ComponentProps<typeof Icon>) => ( | |
| <Icon {...props} name="brain-regular" /> | |
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InfoSheetexpectsicon.Iconto be aReact.ComponentType<IconProps>, so it can passsize,color, etc. The inline components() => <Icon name="..." />ignore those props, which will cause incorrect icon sizing/colors.Replace these with prop-forwarding adapters (e.g.,
(props) => <Icon {...props} name="cocktail-regular" />).