docs: CSS best practices

[jesperhodge]

This is a very early-stage rough draft for an OEP that intends for us to avoid future CSS conflicts.

The motivation comes from bugs I have encountered in the past, for example a CSS file in the header library breaking style in the Authoring MFE, which really shouldn't happen.

There are two solutions that make sense to me: Either a very rudimentary approach of just adopting very minimal rules for how we should scope and implement CSS - by just wrapping it in a unique selector for your component or repo; or adopting CSS Modules.

I lean towards CSS Modules but this is a good start for a discussion.

We can also discuss rejected alternatives; the OEP is just a draft at this point so we can change it in any way the community wishes.

[bradenmacdonald]

Utility-First CSS

I'm a huge fan of Tailwind, and when you build an app with that approach, this problem never happens. We can't adopt Tailwind (and some people might not want to anyways), but we can focus on its style of utility-first CSS as a first step to avoiding this problem. We are already using this pattern a bit in the MFEs and in Paragon, but I personally think we should make more of an effort to use it: Paragon CSS Utility Classes

In other words:

Don't do:

Header.tsx:

import styles from './Header.scss';
const Header = () => {
    return <div className={styles.header}>...</div>
};

Header.scss:

.header {
    margin-bottom: 1em;
    padding-top: 0.25rem;
    padding-bottom: 0.25rem;
}

But instead:

const Header = () => {
    return <div className="mb-3 py-1">...</div>
};

Not only is the second approach much more concise and productive for a developer (no switching between source files), it completely avoids conflicts.

---

As much as you can, I would prefer to use utility classes as a first step to avoiding the issue, and then secondly use CSS modules for styles that cannot be expressed using the utility classes.

Unfortunately, the Paragon utility classes are currently quite limited. I'm used to Tailwind where you can express literally any styling using utility classes (and you get auto-completion of the utility class names in VS Code). The Paragon utility classes on the other hand, are still missing a lot of basic functionality (e.g. try to set a min-width of 50%, or give an element a minimum height on mobile only). So if others like the utility-first approach, I'd love to see an effort to expand the available utility classes in Paragon.

[bradenmacdonald]

CSS Modules

CSS modules is a nice approach, but there's a challenge we face with implementing it: our .scss files usually need to import the paragon SCSS variables/mixins/functions, and doing that in a naive way results in a lot of duplicate SCSS.

For example, learner-dashboard uses some per-page CSS files as one would do when implementing CSS modules, and because each .scss file starts with @import "@openedx/paragon/scss/core/core";, you'll find that there are 10+ separate copies of the full Paragon SCSS rules included in the production build.

In other words:

Don't do:

Header.tsx:

import styles from './Header.scss';
const Header = () => {
    return <>...<div className={styles.something}>...</div></>
};

Header.scss:

@import "@openedx/paragon/scss/core/core";
.something {
    box-shadow: $input-box-shadow;
    max-height: $card-image-vertical-max-height;
    border-radius: $alert-border-radius;
}

But instead:

Header.tsx: same as above.

Header.scss:

@import "@edx/brand/paragon/variables";
@import "@openedx/paragon/scss/core/_functions";
@import "@openedx/paragon/scss/core/_variables";
@import "~bootstrap/scss/mixins";
@import "@openedx/paragon/src/Form/_variables.scss"; // for $input-box-shadow.
@import "@openedx/paragon/src/Card/_variables.scss"; // for $card-image-vertical-max-height
@import "@openedx/paragon/src/Alert/_variables.scss"; // for $alert-border-radius
.something {
    box-shadow: $input-box-shadow;
    max-height: $card-image-vertical-max-height;
    border-radius: $alert-border-radius;
}

---

If we want to support CSS modules (which I think is a nice approach), we need to either use the "one module per app" approach where each MFE has one giant .SCSS entry point for the whole app (example from course-authoring) (ugh), or we need to implement support in Paragon so that you can have many separate .SCSS entry points and they each start with something simple like:

@use "@openedx/paragon/scss/paragon";  // Guaranteed not to emit any rules; this export contains only variables/functions/mixins
.something {
    box-shadow: paragon.$input-box-shadow;
    max-height: paragon.$card-image-vertical-max-height;
    border-radius: paragon.$alert-border-radius;
}

(here I'm also switching to the new @use syntax.)

[jesperhodge]

These are great points. I love the idea of extending Paragon's utility classes. We typically try to use Paragon utility classes when possible, but there are many cases when we do introduce CSS files, and sometimes - if you already have a CSS file - it's just less complicated to do something in there. But that all comes from the utility classes lacking functionality, as you said.

It would be great to compose a list of missing features in Paragon's utility classes and then add classes that are as close to Tailwind as possible. The reason is that Tailwind has proven its ability to do pretty much everything that you can do with CSS in a simple way, so if we can add missing utility's that are closely modeled after tailwind we should have good confidence that we'll have all important features.

Partially because of the need to overwrite child component CSS for paragon components and such from the place you're using it in, I think we'll never reach quite 100% reliance on utility classes but we can get pretty close.

If we go that route, I'm not sure I would additionally recommend CSS Modules. It entails quite a bit of overhead and has the import problems you mentioned.

I think it might be sufficient for the transition period and for CSS that for some reason can't be replaced with utility classes to just put some pretty strict rules in place how your CSS must be scoped. That should avoid most bugs, and if we extend the utility classes and use them everywhere I think the amount of SCSS files we have will be tiny enough that it's unlikely to accidentally break the rules and create a bug.

What do you think @bradenmacdonald ?

[bradenmacdonald]

and then add classes that are as close to Tailwind as possible.

I like that in theory, but it may be a lot of work in practice.

Tailwind also has a lot of combinatorial classes like dark:md:max-xl:hover:bg-yellow-100 (in dark mode, on devices from md to xl size but not smaller or larger, when hovered, use a light yellow background). I think you can use Sass @for loops to generate equivalents, but it's going to produce a ton of classes as the number of combinations get huge. Thus, it might not be practical to replicate entirely. But a simpler subset is probably doable. Either way, it would also require integrating something like PurgeCSS or PurifyCSS to remove unused utility classes from the final build, because there would be a lot. That wouldn't be difficult though.

One way to make this easier could be to find a way to do it incrementally - for example, when someone making an MFE encounters a "missing" utility class that exists in Tailwind but not Paragon, then they add it to mfe/src/scss/new-utilities.scss, and every few months, everything from that file is upstreamed into paragon proper.

I think it might be sufficient for the transition period and for CSS that for some reason can't be replaced with utility classes to just put some pretty strict rules in place how your CSS must be scoped.

That sounds fine to me. Maybe we can even find some existing linter that can help to enforce that to some extent.

[adamstankiewicz]

As much as you can, I would prefer to use utility classes as a first step to avoiding the issue, and then secondly use CSS modules for styles that cannot be expressed using the utility classes.

+1. I agree there's likely opportunities to extend the Paragon utility classes for common use cases beyond what currently exists (most of which are provided by Bootstrap 4), but share @bradenmacdonald's sentiment around being non-trivial / not practical. Also, generally, the CSS utility classes should be used over consuming the SCSS variables as well.

Worth noting, I don't believe the current, default Webpack configuration provided by @openedx/frontend-build supports CSS modules out-of-the-box due to the the css-loader options (i.e., options.modules.compileType: 'icss') [source]). This can be seen when trying to implement CSS modules with frontend-template-application. IIRC, the compileType was set related to Paragon's :export of breakpoints widths from SCSS (to be consumed by breakpoints.js).

I also agree with @jesperhodge's comment below around the overheard of migrating to CSS modules from a code implementation perspective (i.e., introducing className={styles.something} throughout the app):

It entails quite a bit of overhead and has the import problems you mentioned.

CSS modules is a nice approach, but there's a challenge we face with implementing it: our .scss files usually need to import the paragon SCSS variables/mixins/functions, and doing that in a naive way results in a lot of duplicate SCSS.

Regarding the import problems, agreed neither of the workarounds are ideal. I've typically relied on the single SCSS entry point aggregating component-specific SCSS files as that historically has personally felt like a better tradeoff than having multiple copies of Paragon.

That said, it is worth noting that with the (forthcoming) Paragon design tokens work (see ADR, Axim has a funded contribution to get it over the line), Paragon will be less reliant on SCSS variables/mixins in favor of relying on CSS variables instead. This change is to support runtime theming improves support for multi-tenancy and helps set up the (future) capability to have an official dark mode.

As a result, the current import problem with SCSS should be a non-issue in a design tokens / CSS variables world as module-specific CSS/SCSS files in MFEs could reference CSS variables without needing to import anything from Paragon first.

Also worth noting as part of the design tokens project is support for loading the Paragon CSS hosted by a CDN which would ensure a single (cached) copy of Paragon CSS is downloaded by users while navigating across multiple MFEs (kind of like the shared dependencies proposed by OEP-65). Using the CDN approach of consuming Paragon's compiled CSS would be opt-in (i.e., MFEs could still choose to import the Paragon CSS from the NPM package). With OEP-65, the "host" app would likely provide the Paragon CSS variables, but "guest" apps could use CSS variables directly.

Either way, it would also require integrating something like PurgeCSS or PurifyCSS to remove unused utility classes from the final build, because there would be a lot. That wouldn't be difficult though.

[inform] @openedx/frontend-build integrates with PurgeCSS today via the default Webpack configuration (source), enabled via a USE_PURGECSS environment variable when running the build.

IIRC, a handful of MFEs tried to use PurgeCSS to improve performance for similar reasons in the past but ended up disabling it because it ended up being too brittle and often removed important CSS that shouldn't have been removed. Anecdotally, when my team recently focused on frontend performance in frontend-app-learner-portal-enterprise recently, we attempted to enable PurgeCSS and also found it to be too brittle.

Also, related to the aforementioned design tokens project, if MFEs do migrate to consuming Paragon from a CDN instead of importing CSS/SCSS from the @openedx/paragon NPM package like they do today, the Paragon CSS may not go through the Webpack build process where something like PurgeCSS wouldn't access its CSS to do its thing.

I think it might be sufficient for the transition period and for CSS that for some reason can't be replaced with utility classes to just put some pretty strict rules in place how your CSS must be scoped.

That sounds fine to me. Maybe we can even find some existing linter that can help to enforce that to some extent.

Yeah, if there's any ways to help enforce this to some extent, that would be awesome. Unfortunately, I'm not sure the risk of (unintended) conflicts when introducing custom styles is evident to all engineers so making this more obvious at time of implementation would be great; we've had several bugs introduced by multiple engineers who weren't aware of the risk of introducing erroneous CSS conflicts. If nothing else, calling it out explicitly in documentation would be a good start.

That said, this issue of local vs. global scope is the primary advantage/reason for adopting CSS modules so engineers don't need to worry about potential conflicts (i.e., might warrant at least some investigation into how far away we would be from supporting CSS modules, what scope would there be in practice to migrate an MFE to use CSS modules for any of its custom SCSS, etc.).

[jesperhodge]

We continued discussing this in our fedx meeting.

How about something like this?

CSS and SASS Scoping Guidelines

Utility Class Expansion:

• Extend the Paragon utility classes by identifying missing functionalities compared to Tailwind. This includes creating a differential list of features supported by Tailwind versus what's currently supported by Bootstrap 4/Paragon, and incorporating missing features as much as possible.

SASS Declaration and Nesting:

• Adhere to a structured way of declaring and nesting SASS to minimize scope conflicts, as detailed below:

1. Scoped SASS Declarations:

   • Always nest SASS statements within a unique class that specifies the project as well as the component scope: project--component { .my-class { ... } }
   • Example:

     .frontend-app-course-authoring--course-outline { 
     .fancy-button { /* styles */ }
     }

2. Unique Class Names:

   • Ensure the class specifying the scope is always unique, especially when similar components exist within different parts of the application.
   • Example:

     .frontend-app-course-authoring--alert-component-unique { .fancy-button { /* styles */ } }
     .frontend-app-course-authoring--status-bar-alert-component { .fancy-button { /* styles */ } }

3. File Organization:

   • Place each SASS file next to its corresponding component to ensure locality and relevance of styles.

4. Global and Local Styles Distinction:

   • Avoid global style declarations if possible. Use variables, design tokens, and utility classes instead.
   • Global styles must be placed in a specific global SCSS file and should clearly state their scope.
   • Example:

     .frontend-app-course-authoring--global { 
     .my-button { ... }
     }

5. Component-Specific Overrides:

   • When necessary to override styles in a child component or one imported from a library, ensure these styles are scoped to the correct component file.
   • For instance, to change the AlertComponent background color on just the CourseOutline page:

     .frontend-app-course-authoring--course-outline {
     .frontend-app-course-authoring--alert-component { 
      .fancy-button { background-color: yellow; } 
     }
     }

6. Alternative Nesting Pattern:

   • Consider using a project-wide class that nests component-specific classes for broader scope definition.
   • Example:

     .frontend-app-course-authoring {
     .course-outline {
      .alert-component { .fancy-button { /* styles */ } }
     }
     }

Enforcement:

• These rules could be enforced through linting tools where feasible, incorporated into OEPs, or included in PR templates for routine checks during code reviews.

These guidelines are designed to maintain a clear, consistent approach to styling across all our microfrontends, ensuring that styles do not inadvertently affect unrelated parts of the application.

[bradenmacdonald]

@jesperhodge Sounds good 👍🏻