Repository navigation
Conversation
The setting now drives a new --mono-font-family variable so diffs and other font-mono surfaces change, while html/body stay on a hardcoded UI font.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Based on the available options (mostly monospace fonts), it is clear that the "Font Family" setting is intended to control the appearance of code, not the UI chrome around it. This also follows from common sense, as reviewing the code is the primary activity in the app.
However, probably by mistake and low priority, the setting applied to the UI font, so it had very limited utility.
I retired the
--app-font-familyvariable and hardcoded the body styles in the CSS. It is no longer a dynamic style. In its place, I introduced an--mono-font-familythat is forwarded to the--font-monotheme variable used across all monospace/code surfaces. The appearance settings panel is bound to this new variable.Since the setting was used for the UI font, the default value was also proportional/sans while all the other options were mono. I'm guessing this inconsistency is an example of two wrongs that make a right. The default UI font is now exclusively in the CSS and the default value in FONT_FAMILIES is now the tailwind's default
--font-monovalue.Many font presets are hardcoded multiple times across the codebase. I considered unifying it into a single source of truth, but eventually decided against it as it was too invasive in this otherwise simple change.
A migration is needed to adapt existing user settings stored in local storage. Upon read, we try to match user's stored setting with one of the pre-defined options. If that fails, we use the default value. This has the following consequences: