NYPL / nypl-design-system

React design system putting accessibility first.
https://nypl.github.io/nypl-design-system/reservoir/v3/?path=/docs/welcome--docs
Apache License 2.0
71 stars 6 forks source link

CSS Scoping #240

Closed kristojorg closed 3 years ago

kristojorg commented 4 years ago

I have imported styles.scss into my app, and now all of my components are affected by the global styles within. I realize that in some sense this might be desirable, but it can also be quite disruptive. I'm curious what your thoughts are here. One option is to simply stop relying on global selectors like a { /** styles */ }.

What do you all think?

helenvholmes commented 4 years ago

I'd definitely like to namespace all of our styles. Ultimately I'd like to do two things:

  1. Namespace everything with a prefix on compile, something like nds- in front of every className.
  2. Add a hash at the end of all the styles to make conflicts on existing names even less likely, again on compile (probably with something like PostCSS).

Adding the prefix will help people understand that the className is coming from this library. The hash will help with any place where the className is just too common (I'm using nds- as the example proposal here, but even if we, say, did nypl- it's totally possible that a bunch of the apps here would have conflicts in styles for something like .nypl-card.)

@kristojorg thoughts? This is as far as I've gotten thinking through this.

kristojorg commented 4 years ago

Yea I think both of those are a good idea. You can probably accomplish both at build time and just avoid setting global css. You could provide people a class they can put at the top level from where they want the NYPL styles to apply like .nds-root, and then put your design-system global styles in there.

kristojorg commented 4 years ago

You might look at something like css-modules for style encapsulation. I think in general it would also be good to move towards styles separated per-component. So each component would have a styles.scss file next to it in the file tree which would get imported in the component's index.tsx, and with css-modules would be scoped automatically to only apply to that component. It would also means that consumers would only be importing/loading the css that is relevant for the components they're using.

helenvholmes commented 4 years ago

Yeah, a lot of these global styles are holdover from another project that was using a framework that set up those styles. I also really want to move toward style encapsulation per component—the work we did in "Storybook Cleanup" to move everything into scoped folders was in support of that (before it was challenging to even know where you were supposed to put new CSS).

Glad to have my thoughts validated by another person! I am going to have to put this in the Backlog for now but since it's going to continue affecting multiple projects using this stuff I'll see if we can't get this put into the next sprint.

helenvholmes commented 3 years ago

@EdwinGuzman shared this plugin on #489: https://github.com/dbtedman/postcss-prefixwrap

helenvholmes commented 3 years ago

I've been working on this ticket this week and want to give an update as it's kind of a big ticket. Here are my hopes:

Here's some stuff I've noticed:

Here are my outstanding questions:

OR

.nypl-ds-accordion { etc. }

I'm sure there's more that I'm not thinking of, but this is where I'm at with this. I have a linked (still often breaking) PR above. If it all works out, though, I hope this helps us figure out a more modern stack

EdwinGuzman commented 3 years ago

We have some coupled Rails apps/vendors that use the CDN that need all of our CSS. With the changeover, how do we make sure that still works?

Is this because now DS will have a bundled CSS file rather than an SCSS file? Do you know how those apps/vendors already install/use the DS stylesheet file?

helenvholmes commented 3 years ago

Is this because now DS will have a bundled CSS file rather than an SCSS file? Do you know how those apps/vendors already install/use the DS stylesheet file?

DS currently ships a bundled CSS file in the CDN with all of the styles. I just want to preserve that for the CDN even if applications start getting just the CSS they need when they import a component.

helenvholmes commented 3 years ago

Michael copied out my working proposal into a Google doc and left a bunch of comments which I've addressed here.

EdwinGuzman commented 3 years ago

Closing. Will eventually address when we integrate Chakra UI into the codebase.