807 cannot set a number fontweight in typography #1197

swordEdge · 2022-08-19T09:09:50Z

We can't set fontWeight with a number between 100 to 900 which is the most common way to set it.

After fixing:
In Figma, there is a match rule between font family and font-weight, so every font weight doesn't work to every font family.
To test this, we can use Montserrat fontfamily which support all font weights from 100 to 900.

vercel · 2022-08-19T09:09:53Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
figma-tokens	✅ Ready (Inspect)	Visit Preview	Sep 1, 2022 at 1:09PM (UTC)
ft-storybook	✅ Ready (Inspect)	Visit Preview	Sep 1, 2022 at 1:09PM (UTC)

github-actions · 2022-08-19T09:16:56Z

Commit SHA:70e27aec559b28f4d6d057c053389ff202d36366

Test coverage results 🧪

Code coverage diff between base branch:next and head branch: 807-cannot-set-a-number-fontweight-in-typography

Status	File	% Stmts	% Branch	% Funcs	% Lines
🟢	total	68.19 (0)	60.04 (-0.04)	65.2 (0.08)	68.36 (0)
🟢	src/plugin/helpers.ts	80 (1.05)	93.94 (0.19)	100 (0)	80 (1.05)
🟢	src/plugin/setTextValuesOnTarget.ts	100 (3.57)	86.11 (0.82)	100 (0)	100 (3.57)
✨ 🆕	src/plugin/figmaTransforms/fontWeight.ts	36.36	30	100	36.36

LiamMartens · 2022-08-19T13:22:46Z

src/plugin/setValuesOnNode.ts

@@ -29,7 +29,7 @@ export default async function setValuesOnNode(
  const stylePathPrefix = prefixStylesWithThemeName && activeThemeObject
    ? activeThemeObject.name
    : null;
-
+  console.log('values', values);


six7 · 2022-08-19T15:49:07Z

src/plugin/setTextValuesOnTarget.ts

@@ -18,11 +19,12 @@ export default async function setTextValuesOnTarget(target: TextNode | TextStyle
      } = value;
      const family = fontFamily?.toString() || (target.fontName !== figma.mixed ? target.fontName.family : '');
      const style = fontWeight?.toString() || (target.fontName !== figma.mixed ? target.fontName.style : '');
-      await figma.loadFontAsync({ family, style });
+      const transformedValue = transformValue(style, 'fontWeights');


Can we change this so we

First try to set font weight as it is

If that fails try the mapping

Reason is that some (not a lot) fonts have weights actually named '500'

six7 · 2022-08-19T18:12:07Z

This works great so far!

One more thing that I feel would be really great if we could get in, as that way we'd probably be able to cover a lot more font weights

Some fonts define their weights not like SemiBold but Semi Bold (notice the space between Semi and Bold), or sometimes even DemiBold.

Could we pass in our mapping an array of possible weight names? Then we could extend that with some mappings that map in certain font weights. We'd then try to set each of them, and if there's a match, it will apply that?

In the case of 600 this would mean we'd return ['SemiBold', 'Semi Bold', 'DemiBold']? We could use an array for each of these, as I'm sure we'd need to add more possible variations to the other weights later?

github-actions · 2022-08-22T07:05:12Z

Commit SHA:272e6fdba9d8737c3266dc9e01d9814fba531450
Current PR reduces the test coverage percentage by 1 for some tests

six7

Nice! some minor comments

six7 · 2022-08-23T06:55:53Z

src/plugin/setTextValuesOnTarget.test.ts

+    expect(textNodeMock).toEqual({ ...textNodeMock, fontName: { ...textNodeMock.fontName, style: 'Medium' } });
+  });
+
+  it(`can't set number fontWeight to the node if there is no match fontWeight`, async () => {


Suggested change

it(`can't set number fontWeight to the node if there is no match fontWeight`, async () => {

it(`can't set number fontWeight to the node if there is no matching fontWeight`, async () => {

six7 · 2022-08-23T06:56:15Z

src/plugin/setTextValuesOnTarget.test.ts

@@ -35,6 +36,25 @@ describe('setTextValuesOnTarget', () => {
    expect(textNodeMock).toEqual({ ...textNodeMock, fontName: { ...textNodeMock.fontName, style: 'Bold' } });
  });

+  it('convert number fontWeight and sets to the node', async () => {


Suggested change

it('convert number fontWeight and sets to the node', async () => {

it('converts a numerical fontWeight and sets to the node', async () => {

six7 · 2022-08-23T06:57:36Z

src/plugin/setTextValuesOnTarget.ts


+      try {
+        await figma.loadFontAsync({ family, style });


can we move the await inside the if (fontFamily || fontWeight)? Then it will only run if required

six7 · 2022-08-23T06:58:45Z

src/plugin/setTextValuesOnTarget.ts

+          candidateStyles.map(async (candidateStyle) => (
+            await figma.loadFontAsync({ family, style: candidateStyle })
+              .then(() => {
+                if (fontFamily || fontWeight) {


i think we don't even need this check? if we do, it should check for family and candidateStyle - as these are the properties we're setting here.

six7

Small change + can we get test coverage to pass?

six7 · 2022-08-24T06:00:45Z

src/plugin/setTextValuesOnTarget.ts

+          candidateStyles.map(async (candidateStyle) => (
+            figma.loadFontAsync({ family, style: candidateStyle })
+              .then(() => {
+                if (fontFamily || candidateStyle) {


We dont need to check for fontFamily here as we're in an if condition that already checks for that on line 23.

So can just be a check for candidateStyle?

six7 · 2022-08-30T22:01:04Z

Can we still get the test coverage to pass @swordEdge ? Then I'd merge this in 👍

jsamr · 2022-09-21T18:55:17Z

@six7 Sorry to bother you! Has this fix been included in version 119?

six7 · 2022-09-22T19:00:06Z

We released this today with version 120 👍

robinhoodie0823 added 2 commits August 19, 2022 16:03

set number value on fontweight

787bf9d

code refresh

92d68c0

swordEdge linked an issue Aug 19, 2022 that may be closed by this pull request

Cannot set a number fontWeight in typography #807

Closed

vercel bot deployed to Preview – ft-storybook August 19, 2022 09:17 View deployment

vercel bot deployed to Preview – figma-tokens August 19, 2022 09:22 View deployment

swordEdge requested review from six7 and LiamMartens August 19, 2022 10:15

LiamMartens suggested changes Aug 19, 2022

View reviewed changes

six7 requested changes Aug 19, 2022

View reviewed changes

swordEdge requested review from six7 and LiamMartens August 22, 2022 06:49

robinhoodie0823 added 2 commits August 22, 2022 15:54

code refresh

d958f4c

add async/await

9640d99

vercel bot deployed to Preview – figma-tokens August 22, 2022 07:01 View deployment

vercel bot deployed to Preview – ft-storybook August 22, 2022 07:04 View deployment

add test coverage for helper

3b2aadc

vercel bot deployed to Preview – figma-tokens August 22, 2022 09:17 View deployment

vercel bot deployed to Preview – ft-storybook August 22, 2022 09:20 View deployment

add test coverage

e8f9d88

vercel bot deployed to Preview – figma-tokens August 22, 2022 12:54 View deployment

vercel bot deployed to Preview – ft-storybook August 22, 2022 12:56 View deployment

enhance test coverage

a9505f2

vercel bot deployed to Preview – ft-storybook August 22, 2022 13:18 View deployment

vercel bot deployed to Preview – figma-tokens August 22, 2022 13:19 View deployment

six7 requested changes Aug 23, 2022

View reviewed changes

code review

03bb85f

swordEdge requested a review from six7 August 24, 2022 04:58

vercel bot deployed to Preview – figma-tokens August 24, 2022 05:02 View deployment

vercel bot deployed to Preview – ft-storybook August 24, 2022 05:05 View deployment

six7 requested changes Aug 24, 2022

View reviewed changes

swordEdge requested a review from six7 August 24, 2022 06:33

code review

7bc18d8

vercel bot deployed to Preview – figma-tokens August 24, 2022 06:38 View deployment

vercel bot deployed to Preview – ft-storybook August 24, 2022 06:41 View deployment

LiamMartens approved these changes Aug 26, 2022

View reviewed changes

add test coverage

71f6e97

vercel bot deployed to Preview – figma-tokens September 1, 2022 13:07 View deployment

vercel bot deployed to Preview – ft-storybook September 1, 2022 13:09 View deployment

swordEdge requested a review from LiamMartens September 1, 2022 13:13

six7 approved these changes Sep 1, 2022

View reviewed changes

six7 merged commit 53e3d0e into next Sep 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

807 cannot set a number fontweight in typography #1197

807 cannot set a number fontweight in typography #1197

swordEdge commented Aug 19, 2022 •

edited

Loading

vercel bot commented Aug 19, 2022 •

edited

Loading

github-actions bot commented Aug 19, 2022 •

edited

Loading

LiamMartens Aug 19, 2022

six7 Aug 19, 2022

six7 commented Aug 19, 2022

github-actions bot commented Aug 22, 2022 •

edited

Loading

six7 left a comment

six7 Aug 23, 2022

six7 Aug 23, 2022

six7 Aug 23, 2022

six7 Aug 23, 2022

six7 left a comment

six7 Aug 24, 2022

six7 commented Aug 30, 2022

jsamr commented Sep 21, 2022

six7 commented Sep 22, 2022

	it(`can't set number fontWeight to the node if there is no match fontWeight`, async () => {
	it(`can't set number fontWeight to the node if there is no matching fontWeight`, async () => {

	it('convert number fontWeight and sets to the node', async () => {
	it('converts a numerical fontWeight and sets to the node', async () => {

807 cannot set a number fontweight in typography #1197

807 cannot set a number fontweight in typography #1197

Conversation

swordEdge commented Aug 19, 2022 • edited Loading

vercel bot commented Aug 19, 2022 • edited Loading

github-actions bot commented Aug 19, 2022 • edited Loading

Test coverage results 🧪

LiamMartens Aug 19, 2022

Choose a reason for hiding this comment

six7 Aug 19, 2022

Choose a reason for hiding this comment

six7 commented Aug 19, 2022

github-actions bot commented Aug 22, 2022 • edited Loading

six7 left a comment

Choose a reason for hiding this comment

six7 Aug 23, 2022

Choose a reason for hiding this comment

six7 Aug 23, 2022

Choose a reason for hiding this comment

six7 Aug 23, 2022

Choose a reason for hiding this comment

six7 Aug 23, 2022

Choose a reason for hiding this comment

six7 left a comment

Choose a reason for hiding this comment

six7 Aug 24, 2022

Choose a reason for hiding this comment

six7 commented Aug 30, 2022

jsamr commented Sep 21, 2022

six7 commented Sep 22, 2022

swordEdge commented Aug 19, 2022 •

edited

Loading

vercel bot commented Aug 19, 2022 •

edited

Loading

github-actions bot commented Aug 19, 2022 •

edited

Loading

github-actions bot commented Aug 22, 2022 •

edited

Loading