Skip to content
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

feat: generate icons as 16×16 #2837

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/core/generator/fileGenerator.ts
Repiteo marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,11 @@ export const generateFileIcons = async (
}

const fileIcon =
'M13 9h5.5L13 3.5V9M6 2h8l6 6v12a2 2 0 0 1-2 2H6a2 2 0 0 1-2-2V4c0-1.11.89-2 2-2m5 2H6v16h12v-9h-7V4z';
'M9 2v3h3Zm4 2.5V14q0 1-1 1H4q-1 0-1-1V2q0-1 1-1h5.5ZM8 2H4v12h8V6H8Z';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't merge this yet, because it will introduce an inconsistency with the other "file" icons which look simiar:

image

(image is not covering all cases...)

It may sound like perfectionism because it's hard to make out, but I'd prefer consistency over pixel perfectness. If someone has time to bring all files into the same format then we can talk about this, but I'm not feeling comfortable of merging the default file icon with a different shape to the other icons. Sorry for bringing this up so late, but I just noticed it while testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically the original icon WAS pixel-perfect, it was just 24×24. If that's not a problem (or if it's even preferred), I would STRONGLY suggest making the acceptable dimensions explicit in the guidelines; right now it feels like 16×16 or bust

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

24x24 Pixel perfectness will slightly suffer because VS Code renders the icons with 16x16 pixels. That's mentioned in our contribution guidelines.

However, even if it's not pixel perfect for 16x16 pixels it's not such a big problem. The icons are still nice and good to recognize. They can look a little bit blurry then but it's hard to make out the difference. So 24x24 pixel perfectness is still better than no pixel perfectness at all 😅

In general we highly recommend the 16x16 rule for new icons which we create and wherever it's possible. Or to say it in other words: It's more likely to get approval from us. Otherwise we have to evaluate if it's worth adding the icon.

In this case I'd recommend to keep it as it is. Don't spend too much time and energy on that, because we would have to change multiple icons to have the same file icon shape.


await writeSVGFiles(
'file',
getSVG(getPath(fileIcon, color), 24),
getSVG(getPath(fileIcon, color)),
opacity,
saturation
);
Expand Down
14 changes: 6 additions & 8 deletions src/core/generator/folderGenerator.ts
Repiteo marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -450,9 +450,9 @@ export const generateFolderIcons = async (
}

const folderIcon =
'M13.84376,7.53645l-1.28749-1.0729A2,2,0,0,0,11.27591,6H4A2,2,0,0,0,2,8V24a2,2,0,0,0,2,2H28a2,2,0,0,0,2-2V10a2,2,0,0,0-2-2H15.12412A2,2,0,0,1,13.84376,7.53645Z';
'm6.922 3.768-.644-.536A1 1 0 0 0 5.638 3H2a1 1 0 0 0-1 1v8a1 1 0 0 0 1 1h12a1 1 0 0 0 1-1V5a1 1 0 0 0-1-1H7.562a1 1 0 0 1-.64-.232';
const folderIconOpen =
'M28.96692,12H9.44152a2,2,0,0,0-1.89737,1.36754L4,24V10H28a2,2,0,0,0-2-2H15.1241a2,2,0,0,1-1.28038-.46357L12.5563,6.46357A2,2,0,0,0,11.27592,6H4A2,2,0,0,0,2,8V24a2,2,0,0,0,2,2H26l4.80523-11.21213A2,2,0,0,0,28.96692,12Z';
'M14.483 6H4.721a1 1 0 0 0-.949.684L2 12V5h12a1 1 0 0 0-1-1H7.562a1 1 0 0 1-.64-.232l-.644-.536A1 1 0 0 0 5.638 3H2a1 1 0 0 0-1 1v8a1 1 0 0 0 1 1h11l2.403-5.606A1 1 0 0 0 14.483 6';

await writeSVGFiles(
'folder',
Expand Down Expand Up @@ -484,20 +484,18 @@ export const generateRootFolderIcons = async (
return logger.error('Invalid color code for root folder icons');
}

const rootFolderIcon =
'M16,5A11,11,0,1,1,5,16,11.01245,11.01245,0,0,1,16,5m0-3A14,14,0,1,0,30,16,14,14,0,0,0,16,2Zm0,8a6,6,0,1,0,6,6A6,6,0,0,0,16,10Z';
const rootFolderIconOpen =
'M16,5A11,11,0,1,1,5,16,11.01245,11.01245,0,0,1,16,5m0-3A14,14,0,1,0,30,16,14,14,0,0,0,16,2Z';
const rootFolderIconOuter = `<circle cx="8" cy="8" r="6" fill="none" stroke="${color}" stroke-width="2"/>`;
PKief marked this conversation as resolved.
Show resolved Hide resolved
const rootFolderIconInner = `<circle cx="8" cy="8" r="3" fill="${color}"/>`;

await writeSVGFiles(
'folder-root',
getSVG(getPath(rootFolderIcon, color)),
getSVG(rootFolderIconOuter + rootFolderIconInner),
opacity,
saturation
);
await writeSVGFiles(
'folder-root-open',
getSVG(getPath(rootFolderIconOpen, color)),
getSVG(rootFolderIconOuter),
opacity,
saturation
);
Expand Down
2 changes: 1 addition & 1 deletion src/core/generator/shared/svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,5 @@ export const writeSVGFiles = async (
export const getPath = (d: string, color: string) =>
`<path d="${d}" fill="${color}" />`;

export const getSVG = (path: string, viewBoxSize = 32) =>
export const getSVG = (path: string, viewBoxSize = 16) =>
`<svg viewBox="0 0 ${viewBoxSize} ${viewBoxSize}" xmlns="http://www.w3.org/2000/svg">${path}</svg>`;