-
Notifications
You must be signed in to change notification settings - Fork 507
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
Work on implementing code connect to allow for realistic code preview in figma #4545
Conversation
|
3bc79f7
to
2e5be6e
Compare
2b8b5a4
to
77e6ce3
Compare
size-limit report 📦
|
520ad09
to
dc7a1f5
Compare
b6c22dd
to
f3c989b
Compare
3edc831
to
1b87b3b
Compare
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.
Great work @lukasoppermann! Just left some questions 👀
"exclude": ["test/**", "docs/**", "build/**"], | ||
"react": { | ||
"importPaths": { | ||
"./src/*": "@primer/react" |
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.
Would this also work for drafts/experimental/deprecated or do we need to special case those?
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.
Good question, I'd need to test it.
@@ -15,6 +15,7 @@ | |||
"src/utils/story-helpers.tsx", | |||
"**/*.stories.tsx", | |||
"**/*.test.tsx", | |||
"**/*.figma.tsx", |
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.
@lukasoppermann what are you looking for it to be ignored from? Pulled it down real quick and it seemed to be ignored from the build but just wanted to check-in to make sure I knew what you were looking to do 👀
packages/react/tsconfig.json
Outdated
@@ -10,6 +10,7 @@ | |||
"src/**/*.js", | |||
"src/**/*.ts", | |||
"src/**/*.tsx", | |||
"!src/**/*.figma.tsx", |
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.
Let me know if I'm misunderstanding, it seems like we would want this in our type-check
task for CI to make sure that figma connect files are generated correctly but we would not want it in our package files, is that correct?
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.
I think neither.
- def. not in package file
- there are some issues with making it typescript compliant as it is "read as text", this is why I should probably also not be checked.
Happy to jump on a pairing session to see if there is some way around ignoring it.
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.
@lukasoppermann sounds great, would love to pair up on it if you have the time!
… in figma (#4545) * Work on implementing code connect * adding config * fixing lint issues * fix for type issue * fix some icons * fixes for button and adding Link component * add as property to branchname * rm figma:sync script * chore: fix typescript issues in Button.figma for code connect * chore: fix typo * chore: add nocheck for button and generate Octicon figma file * chore: add notice to generated file * fix icons --------- Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
Changelog
figma.config.json
form code connect configpackages/react/package.json
.figma.tsx
file is changedRollout strategy
No effect on package output, just for figma, but it can't be version within figma.
Testing & Reviewing
Merge checklist