search

newscorp-ghfb / NewsKit

The NewsKit Design system
https://newskit.co.uk
Other
130 stars 14 forks source link

Supporting fontMetrics defined in typography-presets.json for Capsize functionality #905

Closed dj-kmuldoon closed 1 year ago

dj-kmuldoon commented 1 year ago

Background

Dow Jones does not plan to upgrade to the v5 version of NewsKit in the near future so will not use Tokens Studio w/ GitHub as a dependency.

Instead, Dow Jones plans to keep the v4 NewsKit going forward while using custom Figma code/tooling developed in-house. Dow Jones has implemented a custom Style Dictionary solution modeled on the v4 NewsKit workflow that takes JSON tokens output from common/open-source Figma plugins (today, 'Design Tokens' from Lukas Opperman) and transforms for iOS (SwiftUI, UIKit), Android (Compose), CSS, SCSS, Email (TypeScript), as well as NewsKit formats for all brands. The advantage for Dow Jones is we are not dependent on custom Figma Plugins developed by Luke Finch which we do not have code access.

The djds-themes-publisher repo is available for anyone within the NewsCorp organization.

Is your feature request related to a problem? Please describe.

Specifically, the data for FontMetrics is currently embedded in 'typography-presets.json' (as spec'd in legacy Dow Jones files) and is not being read by NewsKit and therefore is not transformed via Capsize for TextCrop definitions. Instead, NewsKit is accessing the 'fonts.json' to accomplish this functionality.

Describe the solution you'd like

Solution by updating the NewsKit module (src/utiles/styles/getters.ts) to accept the output of djds-themes-publisher which embeds fontMetrics inside the 'typography-presets.json' AND allow backward compatibility with the current 'NK Theme Exporter' plugin output as developed by Luke Finch.

Describe alternatives you've considered

Dow Jones could modify djds-themes-publisher Style Dictionary to parse and output a 'fonts.json' but we find the abstraction and remapping of a typography-preset item to a fonts item a less than ideal solution. We favor a more explicit declaration in 'typography-presets.json' by either directly embedding or adding a unique key/value dedicated to the fontMetrics for textCrop/Capsize functionality.

Additional context

A sample of current output that works (but manual modification and insertion of fontMetrics is required) and the new djds-themes-publisher output which is completely automated for all variables, including fontMetrics.

//
// typography-presets.json - CURRENT NEWSKIT IMPLEMENTATION
//

"editorialDisplay010": {
  "fontFamily": "{{fonts.fontFamily010.fontFamily}}",
  "fontWeight": "{{fonts.fontWeight020}}",
  "fontSize": "{{fonts.fontSize100}}",
  "lineHeight": "{{fonts.fontLineHeight040}}",
  "letterSpacing": "{{fonts.fontLetterSpacing010}}"
},

Instead, Dow Jones prefers a flatter representation of the NewsKit format data rather than referencing multiple variables in another file. Though we may decide to optimize in the future, today we find managing multiple brands, subbrands and modes for B2B and B2C much easier to visually error-trap when all data is represented in a single file.

//
// typography-presets.json - PREFERRED DOW JONES FORMAT
// Exported via djds-themes-publisher. All typography defined
// within a single file (yes, a bigger file, but with more parameters, 
// and better readability.)
//
"editorialDisplay010": {
  "fontFamily": "Escrow Condensed, Georgia, serif",
  "fontWeight": 700,
  "fontMetrics": {
    "fontWeight010": {
      "capHeight": 689,
      "ascent": 960,
      "descent": -240,
      "lineGap": 0,
      "unitsPerEm": 1000
    }
  },
  "fontSize": "56px",
  "lineHeight": 1.143,
  "letterSpacing": "0%",
  "fontStretch": "normal",
  "textTransform": "none",
  "fontStyle": "normal"
}
mutebg commented 1 year ago

Hi @dj-kmuldoon I am not sure if I understand how you use figma/newskit. Do you export these typography presets from Figma and use your tooling to convert them to the above JSON?

If that's what DowJones need, feel free to open PR, or we can have a call to discuss how to proceed with it.

dj-kmuldoon commented 1 year ago

Do you export these typography presets from Figma and use your tooling to convert them to the above JSON?

why do you want to maintain this typography-preset.json at all?

  • The typography-preset.json contains fontWeight, fontSize, lineHeight, letterSpacing, fontStretch, textTransform, fontStyle in the format defined in the v4 NewsKit (and still needed in v5, I believe). Figma is SSO, but typography-preset.json is simply an output format for NewsKit.

If you manually edit this JSON file after every export...

  • Traditionally, Dow Jones DID manually enter fontMetrics. However, with the new djds-themes-publisher, all is automated. Never a need for manual editing going forward.

I suspect that Dow Jone programmers believed they needed to include the fontMetrics both in the fonts.json and the typography-presets.json, but that isn't true. Only the fonts.json is being read. Yes, I'll reach out and get started on a PR today!

JohnTParsons commented 1 year ago

Source code for Luke's Figma plugin: https://github.com/newscorp-ghfb/newskit-themes-ui Are you sure you can't access it?

dj-kmuldoon commented 1 year ago

THAT I can access, but it may not be the same plugin we've been given at Dow Jones. Certainly, when I requested the source code from Luke Finch, I was informed the code we're using was proprietary and owned by him. The functionality we're reproducing in-house is currently embedded in the 'NK Theme Exporter' plugin which exports tokens in NewsKit format. I'll have a PR up today so you can check out the changes/improvements.

dj-kmuldoon commented 1 year ago

@JohnTParsons @mutebg I'm ready to push up a new branch of NewsKit with changes. All unit tests are passing. Later today, I'll issue a PR with additional tests.

Would the team add me as a contributor to NewsKit so I can push up my branch? Thanks!

JohnTParsons commented 1 year ago

@JohnTParsons @mutebg I'm ready to push up a new branch of NewsKit with changes. All unit tests are passing. Later today, I'll issue a PR with additional tests.

Would the team add me as a contributor to NewsKit so I can push up my branch? Thanks!

To join the newskit-contributors group, go to https://github.com/orgs/newscorp-ghfb/teams/newskit-contributors and click "Request to join". (Everyone in that group has role: Write, which you have already, so I'm not sure what difference it will make.) Write access gives you read, clone, push, manage issues and PRs.

And/or you may want to join https://github.com/orgs/newscorp-ghfb/teams/dj-rendering-devs (which also has Write access).