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

Allow fontFace to accept array of font face rules #1114

Merged

Conversation

taylorfsteele
Copy link
Contributor

This PR adds support to the fontFace function to accept an array of font face rules that share the same font family name.

Example usage:

// text.css.ts
import { fontFace, style } from '@vanilla-extract/css';

const gentium = fontFace([
  {
    src: 'local("Gentium")',
    fontWeight: 'normal',
  },
  {
    src: 'local("Gentium Bold")',
    fontWeight: 'bold',
  },
]);

export const font = style({
  fontFamily: gentium,
});

// CSS
@font-face {
  src: local("Gentium");
  font-weight: normal;
  font-family: "text_gentium__fih47p0";
}
@font-face {
  src: local("Gentium Bold");
  font-weight: bold;
  font-family: "text_gentium__fih47p0";
}
.text_font__fih47p1 {
  font-family: "text_gentium__fih47p0";
}

@changeset-bot
Copy link

changeset-bot bot commented Jun 6, 2023

🦋 Changeset detected

Latest commit: 974d278

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@vanilla-extract/css Minor
@fixtures/features Patch
@fixtures/layers Patch
@fixtures/low-level Patch
@fixtures/recipes Patch
@fixtures/sprinkles Patch
@fixtures/themed Patch
@fixtures/unused-modules Patch
vanilla-extract-example-webpack-react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

If you'd like to define a globally scoped custom font, you can use the "globalFontFace" function instead.
`;

if (Array.isArray(rule)) {
Copy link
Collaborator

@graup graup Jun 7, 2023

Choose a reason for hiding this comment

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

I think you could simplify this code by doing

const rules = Array.isArray(rule) ? rule : [rule];

and then just keeping one code path below.

Copy link
Contributor Author

@taylorfsteele taylorfsteele Jun 7, 2023

Choose a reason for hiding this comment

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

I'm down for that! So we're looking at:

export function fontFace(
  rule: FontFaceRule | FontFaceRule[],
  debugId?: string,
) {
  const fontFamily = `"${cssesc(generateIdentifier(debugId), {
    quotes: 'double',
  })}"`;

  const rules = Array.isArray(rule) ? rule : [rule];

  for (const singleRule of rules) {
    if ('fontFamily' in singleRule) {
      throw new Error(outdent`
      This function creates and returns a hashed font-family name, so the "fontFamily" property should not be provided.
    
      If you'd like to define a globally scoped custom font, you can use the "globalFontFace" function instead.
    `);
    }

    appendCss(
      { type: 'fontFace', rule: { ...singleRule, fontFamily } },
      getFileScope(),
    );
  }

  return fontFamily;
}

I reverted the error string stuff to keep it from where it was previously.

Co-authored-by: Paul Grau <graup@users.noreply.github.com>
@taylorfsteele
Copy link
Contributor Author

@askoufis I saw you ran some CI checks but they were cancelled, anything I need to do on my end?

@askoufis
Copy link
Contributor

@askoufis I saw you ran some CI checks but they were cancelled, anything I need to do on my end?

Not sure why one of them was cancelled. The other one failed because the prettier formatting step failed.

@taylorfsteele
Copy link
Contributor Author

taylorfsteele commented Jun 13, 2023

@askoufis I saw you ran some CI checks but they were cancelled, anything I need to do on my end?

Not sure why one of them was cancelled. The other one failed because the prettier formatting step failed.

Gah, didn't even see the prettier one failed, my bad. Let's see if this latest commit does the trick

@mattcompiles mattcompiles enabled auto-merge (squash) June 19, 2023 01:36
@mattcompiles
Copy link
Contributor

@taylorfsteele Thanks for the PR bud 🍻

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.

5 participants