CSS Reset for hs-app element styles

[justinwolfe]

Problem/Feature

In the mailbox app, we've run into an issue where we'll do our design review based on the Cloudflare deploy preview, get the designer sign-off, and then find that prod doesn't look. Here's an example showing a subtly misaligned label in the Recipients component:

That's caused by this element level styling for label coming in from styles.css in hs-app

We want to be able to trust our deploy previews and not have any surprises for our designers when things show up in prod not looking like what they approved.

Approach

My process (documented here with all the gory details):

• I went through all 28,000 lines of styles.css in hs-app and copied out all element level styles (resulting in ~500 lines left over).
• I did some light editing of things that seemed superfluous/that were themselves resets (resulting in ~400 lines)

Trying to figure out how to condense that, I learned about the all property (MDN, css-tricks) and started out with this approach, targeting all the element level selectors from style.css and resetting them

a, abbr, address, blockquote, body, button, code, dd, dl, dt, form, figure, fieldset, img, input, h1, h2, h3, h4, h5, h6, label, legend, li, ol, pre, svg, table, textarea, ul {
    all: initial;
    display: revert;
}

But it was very clear side-by-side-ing Storybook in local dev with the reset and the published version that that was too aggressive (was making lots of font changes, for example). Since the problems that we've run into so far and I think are most likely to run into are around layout, I then switched to this approach that more specifically targets just those kind of properties:

  a, abbr, address, blockquote, body, button, code, dd, dl, dt, form, figure, fieldset, img, input, h1, h2, h3, h4, h5, h6, label, legend, li, ol, pre, svg, table, textarea, ul {
    margin: initial;
    padding: initial;
    height: initial;
    width: initial;
    line-height: initial;
    border: 0;
  }

That seems okay based on glancing through Storybook, though I'll defer to the experts here about any changes! I also considered a very targeted approach just touching the things we know are causing problems right now, i.e.

label {
    margin: initial,
}

and then adding properties as needed in the future. This seems like the "safest" approach because it limits the surface area of effect from the change, but I'm not sure it's the best option, not because of the need to maintain it and add properties if we see them crop up in ways that impact our designs the future (though that's a thing) but more because only touching the elements we know are causing issues now means we're still going to need to always be double checking our hs-app-ui designs between deploy previews and hs-stack/prod (or not doing that and letting discrepancies slip into prod), whereas if we cover all the element level styles now, we can hopefully have more trust in a 1:1 relationship between the two apps.

That's why I've chosen the second option as the suggestion for this PR, but it's just a suggestion and I defer to you 🧙‍♂️s about what the best approach would be!

Guidelines

• [ ] Follows the established folder/file structure
• [ ] Adds unit tests
• [ ] If it is a refactor or change to an existing component, have you verified it won't break existing Cypress tests or have you updated them?
• [ ] Did you verify some accessibility (a11y) basics?
• [ ] Adds/updates stories. Guidelines
• [ ] Adds/updates documentation (ie proptypes) Guidelines
• [ ] Has it been tested in Help Scout's supported browsers?
• [ ] Requests review from designer of the feature
• [ ] Add label (bug? feature?)

[plbabin]

I love that you list all exploration you did for this issue! I think the latest approach (initial per property) makes sense. We could deploy a beta and have a shot within hs-app and see how that reset work in the wild, I don't think there is a better way to have a sense of the impact of the reset file

[justinwolfe]

@plbabin Thank you!!! Sounds like a great plan—I'll give that a go in the next couple of days!

[cloudflare-pages[bot]]

Deploying with    Cloudflare Pages

Latest commit:	c0e2f42
Status:	✅  Deploy successful!
Preview URL:	https://69ec74b9.hsds-react.pages.dev

View logs

[luketlancaster]

tested the beta in hs-app-ui, both in webpack/dev and inside of the stack:

before (without reset):

after (with reset):

you can see that the thread items now are aligned properly because the margin on the ol from the 'base' styles are being, well, reset 😄

[plbabin]

@luketlancaster do you think it's good enough for applying it everywhere or it should be opt-in for now?

[luketlancaster]

@plbabin oh I guess I should probably test this in hs-app as well 🤔. Lemme do that and get back to you...safest bet would probably be opt-in, how much extra effort do you think that'd be?

[plbabin]

@luketlancaster not that hard. We could add a prop to <HSDS.Provider> and conditionally load the css base on the prop value

[luketlancaster]

@plbabin I poked around for a while in hs-app and didn't notice anything off with this, so I think we should be fine to ship it as is? Though I'll defer to others if we'd rather be safe than sorry

[luketlancaster]

Looks like there are a couple of places in hs-app where this is causing issues (thanks to Ryan for finding these):

1. these illos images in a few places (chat, messages, and search), this is one example (wdp with this beta release on the right, prod on left):

2. The nav at the top, looks like the search autocomplete button/icon is smaller w/these changes (beta release on top, prod on bottom):

so probably a good idea to hide this as much as possible :/

[luketlancaster]

@plbabin / @justinwolfe I'm not super confident that the most recent commit is the correct way to hide these styles behind a prop, but it seemed to work in my testing. It's not super elegant, but it looks to work as expected (at least in the hsds storybook when run locally). Thoughts?

[plbabin]

@plbabin / @justinwolfe I'm not super confident that the most recent commit is the correct way to hide these styles behind a prop, but it seemed to work in my testing. It's not super elegant, but it looks to work as expected (at least in the hsds storybook when run locally). Thoughts?

instead of having to change the value for each property, I extracted the whole reset block to a variable, rest is all good

[luketlancaster]

@plbabin wowowowow that's galaxy brain stuff right there, nice! Gonna push out a beta and take this bad boy for a spin

[luketlancaster]

~So, turns out this doesn't work inside hs-app because the "top" provider (this one is the account dropdown) rules take precedence over the "bottom" (hs-app-ui) one 🤔. Toggling both the cssReset props to true in the devtools makes this work as expected, but I don't think we can do that/it defeats the purpose of this whole dog and pony show anyway.~

edit

I might just be a dummy and have forgotten to npm i...standby

edit 2:

yup I'm for sure a dummy. This works great and y'all should feel great about it. Going to address merge conflicts and get all that sorted

[justinwolfe]

I might just be a dummy and have forgotten to npm i...standby

BEEN THERE

Woohoooooo, thank you for taking this on, so pumped for us not to have to worry about it anymore!!!!