Use CSS Modules in Curriculum Catalog Card

[megcrenshaw]

Uses modules instead of a direct import for styles in the Curriculum Catalog Card.

Tested this manually in storybook:

Links

Testing story

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

• Tests provide adequate coverage
• Privacy and Security impacts have been assessed
• Code is well-commented
• New features are translatable or updates will not break translations
• Relevant documentation has been added or updated
• User impact is well-understood and desirable
• Pull Request is labeled appropriately
• Follow-up work items (including potential tech debt) are tracked and linked

[davidsbailey]

thanks for tackling this! it should be possible to "flatten" the styles in the .module.css file now (see 79e2f04 for example)

[megcrenshaw]

thanks for tackling this! it should be possible to "flatten" the styles in the .module.css file now (see 79e2f04 for example)

Could you share the reasoning for flattening the styles?

Do you mean going from something like

.curriculumCatalogCardContainer {
  ...

  img {
    ...
  }

  .curriculumInfoContainer {
    ...

    .overline {
      ...
    }

    i {
      ...
    }

    .tagsAndTranslatabilityContainer {
      ...

      i {
        ...
      }
    }

to something like

.curriculumCatalogCardContainer {
  ...

  img {
    ...
  }
}

.curriculumInfoContainer {
    ...

    i {
      ...
    }
}

.overline {
      ...
}

.tagsAndTranslatabilityContainer {
      ...

    i {
        ...
    }
}

?

I ask because I feel like the syntax of the module helps us visually decipher which styles are nested and which aren't. The i tags, for example, require the nesting, as some i tags have different styling than others (so I left those nested in the second example). For classes, if we tried to use the .tagsAndTranslatabilityContainer class outside of the .curriculumCatalogCardContainer in the JSX file, it shouldn't work, as the styles written in the CSS file assume that class is nested inside another class –– but we wouldn't know that from looking at the CSS file if the styles were flattened.

I'm not an expert in this stuff, so just trying to understand the reasoning here!

[mgc1194]

Thanks for this Meg 😊

[davidsbailey]

Do you mean going from something like

...

yep! specifically, to un-nest any classnames, since those will be assigned guids:

Could you share the reasoning for flattening the styles?

Sure, two reasons:

1. it seems unnecessary, because now each classname is globally unique, so there's no risk of a nested style getting unintentionally applied due to a css classname collision
2. with classnames being replaced with guids, it seems like there's some increased chance of someone applying the outer style (via styles.curriculumCatalogCardContainer) and expecting the nested styles to get applied to an element with classname 'tagsAndTranslatabilityContainer' (as opposed to styles.tagsAndTranslatabilityContainer)

I ask because I feel like the syntax of the module helps us visually decipher which styles are nested and which aren't. The i tags, for example, require the nesting, as some i tags have different styling than others (so I left those nested in the second example).

agreed, the i tags require nesting. sorry for not being clear about that.

For classes, if we tried to use the .tagsAndTranslatabilityContainer class outside of the .curriculumCatalogCardContainer in the JSX file, it shouldn't work, as the styles written in the CSS file assume that class is nested inside another class –– but we wouldn't know that from looking at the CSS file if the styles were flattened.

what scenario would you say this is guarding against? e.g.

• (a) someone else imports your css module and applies tagsAndTranslatabilityContainer, but not curriculumCatalogCardContainer
• (b) you refactor your own component for tagsAndTranslatabilityContainer to be outside of curriculumCatalogCardContainer

I would argue that we shouldn't worry about (a), and in (b) you should let the .jsx file "own" the layout and not have to worry about restructuring your css when you change the layout in your .jsx file. so I guess unnecessary nesting of css feels like "duplicate state" with respect to what's in the .jsx file.

these are my thoughts... I'm interested if other folks who have been using modules for longer have ideas too.

[davidsbailey]

after talking through it, I think the policy of whether to un-nest classnames is not worth blocking this PR -- but it'd be great if we could figure out a best practice here (possibly including allowing people to do it whichever way they want 😄 )

[megcrenshaw]

@davidsbailey Thanks for the thorough explanation! I'm going to merge this one, and then flatten the stylesheet in a separate PR, mostly to do more thorough comparison between the two