large fb icons only, no density

Summary:
Currently we download a bunch of FB icons and we normally use the smallest one available.

In this diff I change the download logic so we try to download from the largest to the smallest icon and use the first one available. One the client we no longer provide the icon of the same size that is requested, instead we provide the only one we have which will typically be larger than needed. This is a good thing because

1. flipper is a local application and we do not need to worry about icons take up broadband and downloading
2. People have high density displayed

I also stopped using density(rest of related code removed in the next diff) for icons as it the icons themselves did not support it.

Reviewed By: lblasa

Differential Revision: D50495194

fbshipit-source-id: f569c2f3b8ee424a67c6d21136e7e113868b8f6a
This commit is contained in:
Anton Kastritskiy
2023-10-20 07:23:34 -07:00
committed by Facebook GitHub Bot
parent b507f7743e
commit a978c96987
9 changed files with 406 additions and 723 deletions

1
desktop/.gitignore vendored
View File

@@ -12,3 +12,4 @@ tsc-error.log
/flipper-server/static/ /flipper-server/static/
/static/flipper-server-log* /static/flipper-server-log*
/static/.audit.json /static/.audit.json
/static/icons/*_d.png

View File

@@ -21,7 +21,7 @@ test('filled icons get correct local path', () => {
size: 12, size: 12,
density: 2, density: 2,
}); });
expect(iconPath).toBe(path.join('icons', 'star-filled-12@2x.png')); expect(iconPath).toBe(path.join('icons', 'star-filled_d.png'));
}); });
test('outline icons get correct local path', () => { test('outline icons get correct local path', () => {
@@ -31,7 +31,7 @@ test('outline icons get correct local path', () => {
size: 12, size: 12,
density: 2, density: 2,
}); });
expect(iconPath).toBe(path.join('icons', 'star-outline-12@2x.png')); expect(iconPath).toBe(path.join('icons', 'star-outline_d.png'));
}); });
test('filled icons get correct URL', async () => { test('filled icons get correct URL', async () => {
@@ -51,7 +51,7 @@ test('filled icons get correct URL', async () => {
expect(localUrl).toBe(iconUrl); expect(localUrl).toBe(iconUrl);
// ... let's mock a file // ... let's mock a file
const iconPath = path.join(staticPath, 'icons', 'star-filled-12@2x.png'); const iconPath = path.join(staticPath, 'icons', 'star-filled_d.png');
try { try {
await fs.promises.writeFile( await fs.promises.writeFile(
iconPath, iconPath,

View File

@@ -23,7 +23,7 @@ let _icons: Icons | undefined;
function getIconsSync(staticPath: string): Icons { function getIconsSync(staticPath: string): Icons {
return ( return (
_icons! ?? _icons ??
(_icons = JSON.parse( (_icons = JSON.parse(
fs.readFileSync(path.join(staticPath, 'icons.json'), {encoding: 'utf8'}), fs.readFileSync(path.join(staticPath, 'icons.json'), {encoding: 'utf8'}),
)) ))
@@ -31,10 +31,7 @@ function getIconsSync(staticPath: string): Icons {
} }
export function buildLocalIconPath(icon: Icon) { export function buildLocalIconPath(icon: Icon) {
return path.join( return path.join('icons', `${icon.name}-${icon.variant}_d.png`);
'icons',
`${icon.name}-${icon.variant}-${icon.size}@${icon.density}x.png`,
);
} }
export function getLocalIconUrl( export function getLocalIconUrl(

View File

@@ -74,7 +74,7 @@ export function initializeRenderHost(
}, },
getLocalIconUrl(icon, url) { getLocalIconUrl(icon, url) {
if (isProduction()) { if (isProduction()) {
return `icons/${icon.name}-${icon.variant}-${icon.size}@${icon.density}x.png`; return `icons/${icon.name}-${icon.variant}_d.png`;
} }
return url; return url;
}, },

View File

@@ -203,7 +203,7 @@ export function initializeRenderHost(
}, },
getLocalIconUrl(icon, url) { getLocalIconUrl(icon, url) {
if (isProduction()) { if (isProduction()) {
return `icons/${icon.name}-${icon.variant}-${icon.size}@${icon.density}x.png`; return `icons/${icon.name}-${icon.variant}_d.png`;
} }
return url; return url;
}, },

View File

@@ -11,7 +11,7 @@ import React from 'react';
import styled from '@emotion/styled'; import styled from '@emotion/styled';
import {getIconURL} from '../../utils/icons'; import {getIconURL} from '../../utils/icons';
export type IconSize = 8 | 10 | 12 | 16 | 18 | 20 | 24 | 28 | 32; export type IconSize = 8 | 10 | 12 | 16 | 18 | 20 | 24 | 28 | 32 | 48;
const ColoredIconBlack = styled.img<{size: number}>(({size}) => ({ const ColoredIconBlack = styled.img<{size: number}>(({size}) => ({
height: size, height: size,

View File

@@ -10,8 +10,8 @@
import {getRenderHostInstance} from 'flipper-frontend-core'; import {getRenderHostInstance} from 'flipper-frontend-core';
import {IconSize} from '../ui/components/Glyph'; import {IconSize} from '../ui/components/Glyph';
const AVAILABLE_SIZES: IconSize[] = [8, 10, 12, 16, 18, 20, 24, 28, 32]; const AVAILABLE_SIZES: IconSize[] = [8, 10, 12, 16, 18, 20, 24, 28, 32, 48];
const DENSITIES = [1, 2, 3]; const DENSITIES = [1, 1.5, 2, 3, 4];
export type Icon = { export type Icon = {
name: string; name: string;
@@ -58,7 +58,7 @@ function normalizeIcon(icon: Icon): Icon {
}; };
} }
export function getPublicIconUrl({name, variant, size, density}: Icon) { export function getPublicIconUrl({name, variant, size}: Icon) {
return `https://facebook.com/images/assets_DO_NOT_HARDCODE/facebook_icons/${name}_${variant}_${size}.png`; return `https://facebook.com/images/assets_DO_NOT_HARDCODE/facebook_icons/${name}_${variant}_${size}.png`;
} }

View File

@@ -13,7 +13,7 @@ import fetch from '@adobe/node-fetch-retry';
// eslint-disable-next-line node/no-extraneous-import // eslint-disable-next-line node/no-extraneous-import
import type {Icon} from 'flipper-ui-core'; import type {Icon} from 'flipper-ui-core';
const AVAILABLE_SIZES: Icon['size'][] = [8, 10, 12, 16, 18, 20, 24, 28, 32]; const AVAILABLE_SIZES: Icon['size'][] = [8, 10, 12, 16, 18, 20, 24, 28, 32, 48];
export type Icons = { export type Icons = {
[key: string]: Icon['size'][]; [key: string]: Icon['size'][];
@@ -32,38 +32,26 @@ function getIconPartsFromName(icon: string): {
} }
export async function downloadIcons(buildFolder: string) { export async function downloadIcons(buildFolder: string) {
const icons: Icons = JSON.parse( const icons: string[] = JSON.parse(
await fs.promises.readFile(path.join(buildFolder, 'icons.json'), { await fs.promises.readFile(path.join(buildFolder, 'icons.json'), {
encoding: 'utf8', encoding: 'utf8',
}), }),
); );
const iconURLs = Object.entries(icons).reduce<Icon[]>( const iconURLs: Pick<Icon, 'name' | 'variant'>[] = icons.map((rawName) => {
(acc, [entryName, sizes]) => { const {trimmedName: name, variant} = getIconPartsFromName(rawName);
const {trimmedName: name, variant} = getIconPartsFromName(entryName); return {name, variant};
acc.push( });
// get icons in @1x and @2x
...sizes.map((size) => ({name, variant, size, density: 1})),
...sizes.map((size) => ({name, variant, size, density: 2})),
...sizes.map((size) => ({name, variant, size, density: 3})),
);
return acc;
},
[],
);
// Download first largest instance of each icon
await Promise.all( await Promise.all(
iconURLs.map(async (icon) => { iconURLs.map(async (icon) => {
const sizeIndex = AVAILABLE_SIZES.indexOf(icon.size); const sizesToTry = [...AVAILABLE_SIZES];
if (sizeIndex === -1) {
throw new Error('Size unavailable: ' + icon.size);
}
const sizesToTry = AVAILABLE_SIZES.slice(sizeIndex);
while (sizesToTry.length) { while (sizesToTry.length) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const size = sizesToTry.shift()!; const size = sizesToTry.pop()!;
const url = getPublicIconUrl({...icon, size}); const url = getPublicIconUrl({...(icon as Icon), size});
const res = await fetch(url); const res = await fetch(url);
if (res.status !== 200) { if (res.status !== 200) {
// console.log( // console.log(
@@ -89,21 +77,21 @@ export async function downloadIcons(buildFolder: string) {
console.error( console.error(
`Could not download the icon ${JSON.stringify( `Could not download the icon ${JSON.stringify(
icon, icon,
)} from ${getPublicIconUrl(icon)}, didn't find any matching size`, )} from ${getPublicIconUrl({
...icon,
size: AVAILABLE_SIZES[AVAILABLE_SIZES.length - 1],
} as Icon)}, didn't find any matching size`,
); );
}), }),
); );
} }
// should match flipper-ui-core/src/utils/icons.tsx // should match flipper-ui-core/src/utils/icons.tsx
export function getPublicIconUrl({name, variant, size, density}: Icon) { export function getPublicIconUrl({name, variant, size}: Icon) {
return `https://facebook.com/images/assets_DO_NOT_HARDCODE/facebook_icons/${name}_${variant}_${size}.png`; return `https://facebook.com/images/assets_DO_NOT_HARDCODE/facebook_icons/${name}_${variant}_${size}.png`;
} }
// should match app/src/utils/icons.tsx // should match app/src/utils/icons.tsx
function buildLocalIconPath(icon: Icon) { function buildLocalIconPath(icon: Pick<Icon, 'name' | 'variant'>) {
return path.join( return path.join('icons', `${icon.name}-${icon.variant}_d.png`);
'icons',
`${icon.name}-${icon.variant}-${icon.size}@${icon.density}x.png`,
);
} }

File diff suppressed because it is too large Load Diff