Styling: Fix text styles

thansidwell commented 9 months ago

Right now the text appears in all different sizes which gives an uneven, haphazard feeling. https://share.cleanshot.com/wSCPdJbF

We need to add a type scale system and text styles.

Here's the type scale we can use. I built it with an online tool. Open link.
This type scale dynmatically changes size depending on the size of the device that's using it. It's a more elegant alternative to breadpoint changes. https://share.cleanshot.com/mMV25cpW
It used CSS clamp which seems to have sufficent support. https://share.cleanshot.com/wXK6sJP0
Here's the text styles I'd like to use, built on that type scale. https://share.cleanshot.com/qNjSzRNf
I roughed out what the CSS might be. text-styles.css.zip

Next, apply these classes all throughout the site.

Landing page https://share.cleanshot.com/7zJhPJnB
Find Properties https://share.cleanshot.com/dpV4xQWH
Filter https://share.cleanshot.com/fFfg0gPR
Property https://share.cleanshot.com/s1dbbFDX
Content pages (Get Access, Transform a Property, Summary, Methodology) https://share.cleanshot.com/1X6mPr80

Here's the CSS to support this.

/* Fluid font size variables, for browsers that support clamp */

@supports (font-size: clamp(1rem, 1vw, 1rem)) {
  :root {
    --font-size-sm: clamp(0.89rem, -0.22vw + 0.94rem, 0.77rem);
        --font-size-md: clamp(1rem, 0vw + 1rem, 1rem);
        --font-size-lg: clamp(1.13rem, 0.32vw + 1.05rem, 1.3rem);
        --font-size-xl: clamp(1.27rem, 0.77vw + 1.07rem, 1.69rem);
        --font-size-2xl: clamp(1.42rem, 1.41vw + 1.07rem, 2.2rem);
        --font-size-3xl: clamp(1.6rem, 2.28vw + 1.03rem, 2.86rem);
  }
}
/* Fallback variables for browsers that don't support clamp */
@supports not (font-size: clamp(1rem, 1vw, 1rem)) {
  :root {
    --font-size-sm: 0.89rem;
        --font-size-md: 1rem;
        --font-size-lg: 1.13rem;
        --font-size-xl: 1.27rem;
        --font-size-2xl: 1.42rem;
        --font-size-3xl: 1.6rem;
  }
  @media screen and (min-width: 1280px) {
    :root {
      --font-size-sm: 0.77rem;
            --font-size-md: 1rem;
            --font-size-lg: 1.3rem;
            --font-size-xl: 1.69rem;
            --font-size-2xl: 2.2rem;
            --font-size-3xl: 2.86rem;
    }
  }
}
.heading-3xl {
    font-size: var(--font-size-3xl);
    font-family: var(--font-family);
    font-weight: 800;
}
.heading-2xl {
    font-size: var(--font-size-2xl);
    font-family: var(--font-family);
    font-weight: 800;
}
.heading-xl {
    font-size: var(--font-size-xl);
    font-family: var(--font-family);
    font-weight: bold;
}
.heading-lg {
    font-size: var(--font-size-lg);
    font-family: var(--font-family);
    font-weight: bold;
}
.heading-md {
    font-size: var(--font-size-md);
    font-family: var(--font-family);
    font-weight: bold;
}
.body-lg {
    font-size: var(--font-size-lg);
    font-family: var(--font-family);
    font-weight: regular;
}
.body-md {
    font-size: var(--font-size-md);
    font-family: var(--font-family);
    font-weight: regular;
}
.body-sm {
    font-size: var(--font-size-sm);
    font-family: var(--font-family);
    font-weight: regular;
}

paulhchoi commented 8 months ago

@thansidwell This is likely a question for this project's lead dev, but what is the team's appetite for adding enforcement rules via Stylelint or ESLint to explicitly disallow tailwinds defaults of text-* or even these font class additions of font-size-*? My thought is that while we could add in the heading-* and body-* classes as you prescribe here, without enforcing disallows through linting rules of at least text-*, I'd fear that contributors in the future would just use text-* classes as they always would for any other project with tailwind, causing future styling inconsistencies.

brandonfcohen1 commented 8 months ago

@thansidwell This is likely a question for this project's lead dev, but what is the team's appetite for adding enforcement rules via Stylelint or ESLint to explicitly disallow tailwinds defaults of text-* or even these font class additions of font-size-*? My thought is that while we could add in the heading-* and body-* classes as you prescribe here, without enforcing disallows through linting rules of at least text-*, I'd fear that contributors in the future would just use text-* classes as they always would for any other project with tailwind, causing future styling inconsistencies.

love this idea, are you able to take the lead on this?

paulhchoi commented 8 months ago

Just added the changes in the PR above.

Couple things to note were:

@thansidwell while the PR made the requested wholesale changes throughout the app, it may be cleaner to simply map html elements to heading or body classes (eg: all <h1>'s should be heading-xl's, all <p>'s should be body-md's) — if this mapping were provided, we could just define the css once in the global.css file, and not use any size classes in any of the copy elements, except for one-off cases.
@brandonfcohen1 this project doesn't seem to have any linting enforced — it may be a good idea to add at a minimum the react and next/core-web-vitals rules to make sure contributors aren't swapping codestyles based on their own local settings (I think I saw a PR earlier that was mostly just single-to-double-qoute changes). The .eslintrc.json file I added with this PR would make it easy to add as you'd just need to add the appropriate extends:{} and plugins: [], but I figured I should leave that off in case there were team preferences for linting styles.

thansidwell commented 8 months ago

Hey @paulhchoi and @brandonfcohen1! I just found time to look at the preview. It looks great! I don't see any issues.

@paulhchoi To your question about whether to style by class like "heading-xl" or by HTML tag/element like h1, I've seen most use classes, in design systems and frameworks. I assume it allows for more flexibility. Maybe you want something to be an h1 for search engine purposes, but you want it to look like a small heading, like a heading-s.

paulhchoi commented 8 months ago

@thansidwell gotcha yea I can align with that 👍

thansidwell commented 8 months ago

@paulhchoi So it looks like these changes are on staging. The headings resize beautifully.

One small issue I found is that the text on the map legend became even smaller. https://share.cleanshot.com/2N94qLst Could you quickly fix that or should we make a new ticket?

thansidwell commented 8 months ago

@paulhchoi I'll just write a new ticket to fix the text styles on the map legend.

paulhchoi commented 8 months ago

@paulhchoi I'll just write a new ticket to fix the text styles on the map legend.

ah sorry about this miss, opening a PR for this now

CodeForPhilly / clean-and-green-philly

Styling: Fix text styles #279