bcgov / bc-sans

Npm package for official bc gov web font
Apache License 2.0
16 stars 5 forks source link

Add @font-face declarations using 'BC Sans' for font-family #12

Closed ty2k closed 10 months ago

ty2k commented 10 months ago

I'm making this PR for discussion - there might be a better way to solve this that I'm missing.

The CSS file that ships with this package has always used BCSans (no space) for the font-family value inside its @font-face declarations. By contrast, the metadata in the font files use BC Sans (with a space) for the font family name.

Here's an example of the metadata for the WOFF file for BC Sans Regular in a Font Forge window:

Font Forge windows showing BC Sans font family name

We (OSS Design Team in GDX) have begun work on a new version of the BC Design System where we will be shipping a Figma design system file and a design tokens package for use by designers and developers. Figma alows for designers to design using custom fonts like BC Sans, either through its desktop app directly, or through its web interface using a separate desktop font integration app. Since the BC Sans font files use the name BC Sans with a space, all designs that reference the font end up using the name with the space in the code samples generated by Figma. Because of this, if I want to allow our @bcgov/design-tokens package to work with the CSS provided by this package, I would need to manually update the name of the font-family in the design tokens variables.

Example design token CSS variable using code as it comes out of Figma:

--bcds-typography-regular-body: 400 1rem/1.688rem 'BC Sans';

If I were to use that CSS variable along with the CSS @font-face code in this package as it is today, my end users would not see the BC Sans font as intended.

Any Figma user designing with BC Sans will have this issue, regardless of whether they choose to use our design tokens or not. Here's an example new Figma file that doesn't use the design tokens, which I've built using the web interface. I've set the font face to BC Sans using Figma's font agent on my Mac:

Figma file showing BC Sans usage

To improve the designer and developer experience of using this package along with Figma, I propose that we should add the correct font name to the CSS declarations as I've done in d13e52f47d478dafb491a85623321dc0fc67daec. I don't think we can make the correction in place, because many apps use this package and have code written to reference the BCSans name. Even fixing it in place with a major version bump will cause a lot of pain. I think any change like this would need to be additive.

I recognize that this will mean more code shipped to the end user. However, I feel the benefit in terms of designer and developer experience are worth the extra 1.2K. Figma has been increasing in popularity to the point where it's the most popular UI prototyping software, and it has been requested by our design community for years. Without this change, any future designer/developer teams using Figma to generate code in our ecosystem would also need to be aware of this bug, manually updating their code. It's also not easy for employees on Gov-provisioned laptops to notice that there is a problem with Figma's generated code - assuming they have the BC Sans font installed, the reference to BC Sans from Figma's code will work with their system fonts, and the problem might make it to the end user.

Please let me know your thoughts, @ShawnTurple , @mhaswell-bcgov .

ty2k commented 10 months ago

After talking with @ShawnTurple, he suggested adding the new @font-declaration rules to a new CSS file. I've done this in 1129131a479d4c7bfac8b4f0147b0e1e1057c42e. This will allow new projects to use the new CSS file while existing projects can continue using the old one, minimizing their bundle sizes.

I've updated the README in c53455d942bb1127738fe1642a9ebee2e008334d. I am referencing the new css/BC_Sans.css file as the default in the existing code samples. I've added a section on why the two files exist, aiming for a grade 7 reading level on Hemingway.

ty2k commented 10 months ago

I have bumped the package version to v2.1.0 in 4d961d438b947483b78496d3042d835176edb480.