Refactor internal typography theme data structure
What is the purpose of this change?
We'd like to simplify the internals of our Typography infrastructure as it has been noted over time that it's becoming quite difficult for developers unfamiliar with the codebase to make modifications and, more generally, understand what is going on.
Before beginning this work, we spent some time digging into the reasons why developers might find it difficult to approach this part of our codebase and whittled it down to a few key areas that this sequence of changes aims to address.
The following list captures the set of this list that this pull request covers:
- A level of indirection exists in our typography theme token definitions; making it hard to quickly find the value required because of a mapping from each value to an array containing the scale.
- The file also seemed large with some unnecessary abstractions and comments describing the mapping instead of it being described statically in our code.
- In some areas such as our definition for the
remfont size scale, the values are not statically defined and are instead computed at runtime, meaning that accessing the final computed value through static analysis in the editor was not possible. - We noticed that in some areas, the code could be better documented
This is the first set of changes captured by that list. We plan to follow this PR with some further changes to how we internally compute and define the final font styles (specifically focusing on the fs method of legend and how we expose the font style both as a string and as an object)
What does this change?
The set of changes captured in this PR seeks to address the items raised in the list above.
The list below provides a brief overview of how the changes made, and how they relate to the points raised in the list above.
Flattening of the token data structure
Each group of design tokens (fontWeights, lineHeights, textSizes, etc...) was defined as a two-level mapping; with a lookup pulling each value from an array of every possible value in the given scale.
To illustrate with an example. A lot of our mappings looked like this:
const fontSizes = [12, 14, 15, 17, 20, 24, 28, 34, 42, 50, 70] as const;
const titlepieceSizes = {
small: fontSizes[8], //42px
medium: fontSizes[9], //50px
large: fontSizes[10], //70px
} as const;
We noticed that we've introduced a potentially confusing of abstraction here. Over time as the file grew, more font definitions were added, creating a scenario in which developers were not able to easily determine what the value for each of the size aliases was without an adjacent comment in place to stop the necessity to continually scroll back up to refer back to the fontSizes scale.
This mapping is liable to easily fall out of date over time and potentially cause real confusion due to simple human error, because we have no way of checking to see if the comments are correct.
To address this, we instead take the step of concretely defining the values for each size alias at the point of defining the mapping. The benefit of this approach is that the values are kept in the same place as the alias at the point of definition, reducing mental load on the developer when they need to understand/modify an element of the file.
A downside of this approach is that we lose our tight adherence to a well defined scale, as the fontSizes array ceases to exist. This can be remedied at the level of our type checker, ensuring that any values entered into the data structure are valid and belong to the scale as defined in our design system.
(TODO: can we enforce scale membership at a type level?)
🦋 Changeset detected
Latest commit: c2c3b5f56bd950dd429d34608ee7837d4b0e4b52
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 1 package
| Name | Type |
|---|---|
| @guardian/source-foundations | 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
The followup PR is a work in progress here if of interest :) https://github.com/guardian/source/pull/1525 feedback welcome!