Skip to content

feat(CC-icons): added support scripts #11794

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mattnolting
Copy link
Contributor

@mattnolting mattnolting commented Apr 29, 2025

closes: #11620

Installation instructions

  1. Generate a figma access token

Screenshot 2025-05-06 at 12 20 04 PM

Screenshot 2025-05-06 at 12 22 11 PM

Screenshot 2025-05-06 at 12 24 59 PM

Then copy the token and save somewhere safe.

  1. Follow instructions in GETTING-STARTED.md

This PR provides all necessary files to:

  • Update iconsData.json
  • Sync and connect figma icon instances

In following PRs:

  • URIs should be centralized as file paths change between Figma branches

@patternfly-build
Copy link
Contributor

patternfly-build commented Apr 30, 2025

'Font size - 2XL': 'font-size---2xl',
'Font size - 3XL': 'font-size---3xl',
'Font size - 4XL': 'font-size---4xl'
})
Copy link
Member

Choose a reason for hiding this comment

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

Add the children prop as the actual icon displayed is passed as a child of the component.

Suggested change
})
}),
children: figma.children('*')

Copy link
Member

Choose a reason for hiding this comment

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

Talking about changing this from children to instance...

Copy link
Member

Choose a reason for hiding this comment

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

Add to follow-up issue

// Read the existing icons data
let config;
try {
const configPath = path.resolve(process.cwd(), 'codeConnect/config.json');
Copy link
Member

Choose a reason for hiding this comment

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

This file doesn't exist so the code always falls to the catch, should this file be part of this PR?

Copy link
Member

Choose a reason for hiding this comment

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

No extra file needed - we'll open follow-up issue to review & potentially remove this unused code.

const importStatement = `import React from "react";
import {
${componentNames.join(',\n ')}
} from "@patternfly/react-icons/dist/esm/icons";
Copy link
Member

Choose a reason for hiding this comment

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

FYI running the script, this imports all icons from this one import path as seen below:

  WindowsIcon,
  WrenchIcon,
  ZoneIcon
} from "@patternfly/react-icons/dist/esm/icons";

The existing icons.figma.tsx file in this PR has separate imports as so:

import WindowsIcon from './@patternfly/react-icons/dist/esm/icons/windows-icon';
import WrenchIcon from './@patternfly/react-icons/dist/esm/icons/wrench-icon';
import ZoneIcon from './@patternfly/react-icons/dist/esm/icons/zone-icon';

@evwilkin
Copy link
Member

evwilkin commented Jun 2, 2025

Mostly small changes that should be quick, but we should also make the codeConnect a sibling directory to react-core to match the structure from #11827 & related component PRs.

FYI - for this PR the API call to fetch icons is erroring out, I can confirm this was working previously but for now we can use the hard-coded icons data that you've included and address this in a follow-up enhancement.

@@ -0,0 +1,31 @@
import figma from '@figma/code-connect';
Copy link
Member

Choose a reason for hiding this comment

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

Not this line but the file name - missed earlier that the file can be renamed from "IconWrapper" to just "Icon" to match the component name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feat - Figma Icon Connect
3 participants