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.

Sign up for GitHub

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

Jump to bottom

Replace Storybook icon docs with icon table (including metadata) #1260

Closed
wants to merge 7 commits into from
Closed

Replace Storybook icon docs with icon table (including metadata) #1260

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9372da3
Replace icon docs with generated icon table
fredvisser May 19, 2023
188e283
Change files
fredvisser May 19, 2023
5420654
review feedback
fredvisser May 22, 2023
31de1c1
review feedback
fredvisser May 22, 2023
46e1731
build issue fix attempt
fredvisser May 22, 2023
2d8b3f2
Merge branch 'main' into users/fvisser/icon-gallery-with-metadata
fredvisser May 24, 2023
dee963b
explicit import
fredvisser May 24, 2023
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
7 changes: 7 additions & 0 deletions change/@ni-nimble-components-897a77b2-4280-475f-977b-21a0571b7acf.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be a good topic for our next call with our Chromatic rep. If you agree, it'd be good to save off some links/screenshots/chromatic builds/notes to show why we didn't end up using the icon gallery doc block.

"type": "none",
"comment": "Replace icon docs with generated icon table",
"packageName": "@ni/nimble-components",
"email": "1458528+fredvisser@users.noreply.github.com",
"dependentChangeType": "none"
}
30 changes: 29 additions & 1 deletion packages/nimble-components/build/generate-icons/source/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
* Build script for generating Nimble components for icons.
*
* Iterates through icons provided by nimble-tokens, and generates a Nimble component for each in
* src/icons. Also generates an all-icons barrel file.
* src/icons. Also generates an all-icons barrel file and the icons.mdx Storybook file.
*/
import { pascalCase, spinalCase } from '@microsoft/fast-web-utilities';
import * as icons from '@ni/nimble-tokens/dist/icons/js';
import { iconMetadata } from '../../../dist/esm/icon-base/icon-metadata';

const fs = require('fs');
const path = require('path');
Expand All @@ -18,6 +19,18 @@ const trimSizeFromName = text => {
const generatedFilePrefix = `// AUTO-GENERATED FILE - DO NOT EDIT DIRECTLY
// See generation source in nimble-components/build/generate-icons\n`;

const mdxFilePrefix = `import { Meta, Title, IconItem } from '@storybook/blocks';
import * as iconStories from '../icon-base/tests/icons.stories';

<Meta of={iconStories} />
<Title of={iconStories} />
`;

const mdxIconTablePrefix = `
| Icon | Synonyms |
| ---- | -------- |
`;

const iconsDirectory = path.resolve(__dirname, '../../../src/icons');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put the icons.mdx file in the top-level icons directory, instead of a tests directory. Since the whole folder is autogenerated, maybe that's fine?

Copy link
Member

Choose a reason for hiding this comment

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

The files generated should be treated as source code. I.e. could turn off the generation and check them in. So it's fine to be in icons but should go under icons/tests.

Reason behind the reason is files in any folder named tests/ get excluded from the final package automatically, so lean into that convention.

Copy link
Member

@rajsite rajsite May 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Can see Malcolm's script for setting up the directory structure:

nimble/packages/nimble-blazor/build/generate-playwright-version-properties/source/index.js

Line 11 in 4bbe120

const destAbsoluteDir = path.resolve(__dirname, '../dist/');


if (fs.existsSync(iconsDirectory)) {
Expand All @@ -31,7 +44,10 @@ console.log('Finished creating icons directory');

console.log('Writing icon component files');
let allIconsFileContents = `${generatedFilePrefix}\n`;
let mdxFileContents = `${mdxFilePrefix}\n`;
let mdxTableFileContents = `${mdxIconTablePrefix}`;
let fileCount = 0;

for (const key of Object.keys(icons)) {
const svgName = key; // e.g. "arrowExpanderLeft16X16"
const iconName = trimSizeFromName(key); // e.g. "arrowExpanderLeft"
Expand All @@ -41,6 +57,10 @@ for (const key of Object.keys(icons)) {
const className = `Icon${pascalCase(iconName)}`; // e.g. "IconArrowExpanderLeft"
const tagName = `icon${pascalCase(iconName)}Tag`; // e.g. "iconArrowExpanderLeftTag"

const iconSynonyms = iconMetadata[className];
const mdxImportStatements = `import { ${tagName} } from './${fileName}';`;
const mdxIconTableRow = `| <IconItem name="${fileName}"><${elementName} /></IconItem> | ${iconSynonyms?.tags.join(', ')} |`;

const componentFileContents = `${generatedFilePrefix}
import { ${svgName} } from '@ni/nimble-tokens/dist/icons/js';
import { DesignSystem } from '@microsoft/fast-foundation';
Expand Down Expand Up @@ -70,10 +90,18 @@ export const ${tagName} = DesignSystem.tagFor(${className});
fileCount += 1;

allIconsFileContents = allIconsFileContents.concat(`export { ${className} } from './${fileName}';\n`);
mdxFileContents = mdxFileContents.concat(`${mdxImportStatements}\n`);
mdxTableFileContents = mdxTableFileContents.concat(`${mdxIconTableRow}\n`);
}
console.log(`Finshed writing ${fileCount} icon component files`);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be easier to follow the output document structure if we kept all the MDX stuff together inside the loop and concatenated the parts of the output file all at once after the loop. So something more like this:

let mdxImportStatements = '';
let mdxTable = mdxIconTablePrefix;
for (const key of Object.keys(icons)) {
    // ...
    mdxImportStatements += `import { ${tagName} } from './${fileName}';\n`;
    mdxTable += `| <IconItem name="${fileName}"><${elementName} /></IconItem> | ${iconSynonyms?.tags.join(', ')} |\n`;
    // ...
}

const mdxFileContents = `
${mdxFilePrefix}
${mdxImportStatements}
${mdxTable}`;


const allIconsFilePath = path.resolve(iconsDirectory, 'all-icons.ts');
console.log('Writing all-icons file');
fs.writeFileSync(allIconsFilePath, allIconsFileContents, { encoding: 'utf-8' });
console.log('Finished writing all-icons file');

const iconsMDXFilePath = path.resolve(iconsDirectory, 'icons.mdx');
console.log('Writing icons.mdx file');
mdxFileContents = mdxFileContents.concat(`${mdxTableFileContents}`);
fs.writeFileSync(iconsMDXFilePath, mdxFileContents, { encoding: 'utf-8' });
console.log('Finished writing icons.mdx file');
3 changes: 1 addition & 2 deletions packages/nimble-components/src/icon-base/tests/icons.stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ interface IconArgs {
}

const metadata: Meta<IconArgs> = {
title: 'Icons',
tags: ['autodocs']
title: 'Icons'
};

export default metadata;
Expand Down