Skip to content

Conversation

@michaelchadwick
Copy link
Contributor

Refs ilios/ilios#6608
Fixes ilios/ilios#6710

This is a biggie, and will be done in several passes:

  1. "easy" fixes where the mixin does not mince words with another mixin, either under or alongside
  2. "medium" fixes where the mixin targets an element that never shows up in a template (cruft?)
  3. "hard" fixes where the mixin does mingle with another mixin, either under or alongside

I'm trying to keep commits generally within those boundaries so my journey can be properly followed along with.

@netlify
Copy link

netlify bot commented Dec 5, 2025

Deploy Preview for ilios-frontend ready!

Name Link
🔨 Latest commit 07fbf18
🔍 Latest deploy log https://app.netlify.com/projects/ilios-frontend/deploys/69458a4e463d18000851fb8e
😎 Deploy Preview https://deploy-preview-8997--ilios-frontend.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@michaelchadwick michaelchadwick force-pushed the 6710-replace-font-size-sass-mixin branch from f771fe7 to 0437cb5 Compare December 5, 2025 19:42
@michaelchadwick
Copy link
Contributor Author

The mixin has been extinguished, but the second-to-last commit, uh, commits a grievous error: simply adding the new shared class's attributes to a bunch of deeply-nested places that I couldn't yet figure out how to untangle.

@michaelchadwick michaelchadwick force-pushed the 6710-replace-font-size-sass-mixin branch 2 times, most recently from 9a386ed to 941140e Compare December 12, 2025 21:12
@michaelchadwick michaelchadwick changed the title Replace font-size($step) SASS mixin with equivalent CSS classes Mostly replace font-size($step) SASS mixin with equivalent CSS classes Dec 12, 2025
@michaelchadwick michaelchadwick force-pushed the 6710-replace-font-size-sass-mixin branch from 941140e to 229f6df Compare December 12, 2025 21:58
@michaelchadwick michaelchadwick marked this pull request as ready for review December 15, 2025 18:51
@stopfstedt stopfstedt added the run ui tests Run the expensive UI tests label Dec 17, 2025
@stopfstedt
Copy link
Member

the remaining usage of these mixins.

stefan@nichtsnutz: ~/dev/projects/frontend on 6710-replace-font-size-sass-mixin[$]
$ grep -r '@include' packages --exclude-dir=dist --exclude-dir=node_modules | grep font-size
packages/ilios-common/app/styles/ilios-common/components/dashboard/navigation.scss:          @include m.font-size("large");
packages/ilios-common/app/styles/ilios-common/components/dashboard/navigation.scss:            @include m.font-size("medium");
packages/ilios-common/app/styles/ilios-common/shared/main-list-box-header-title.scss:  @include font-size.text-align-bottom;
packages/ilios-common/app/styles/ilios-common/mixins/course/header.scss:    @include font-size.text-align-bottom;
packages/ilios-common/app/styles/ilios-common/mixins/course/header.scss:      @include font-size.text-align-bottom;
packages/ilios-common/app/styles/ilios-common/mixins/ilios-heading.scss:  @include font-size.font-size("xxxl");
packages/ilios-common/app/styles/ilios-common/mixins/ilios-heading.scss:  @include font-size.font-size("xxl");
packages/ilios-common/app/styles/ilios-common/mixins/ilios-heading.scss:  @include font-size.font-size("xl");
packages/ilios-common/app/styles/ilios-common/mixins/ilios-heading.scss:  @include font-size.font-size("large");
packages/ilios-common/app/styles/ilios-common/mixins/ilios-heading.scss:  @include font-size.font-size("medium");
packages/ilios-common/app/styles/ilios-common/mixins/ilios-heading.scss:  @include font-size.font-size("base");
packages/ilios-common/app/styles/ilios-common/mixins/ilios-button.scss:  @include font-size.font-size("base");
packages/ilios-common/app/styles/ilios-common/mixins/ilios-select.scss:  @include font-size.font-size("base");
packages/frontend/app/styles/components/ilios-navigation.scss:        @include m.font-size("small");
stefan@nichtsnutz: ~/dev/projects/frontend on 6710-replace-font-size-sass-mixin[$]
$ grep -r '@include' packages --exclude-dir=dist --exclude-dir=node_modules | grep font-size | sed 's/:\(.*\)//' | sort -u                                                          
packages/frontend/app/styles/components/ilios-navigation.scss
packages/ilios-common/app/styles/ilios-common/components/dashboard/navigation.scss
packages/ilios-common/app/styles/ilios-common/mixins/course/header.scss
packages/ilios-common/app/styles/ilios-common/mixins/ilios-button.scss
packages/ilios-common/app/styles/ilios-common/mixins/ilios-heading.scss
packages/ilios-common/app/styles/ilios-common/mixins/ilios-select.scss
packages/ilios-common/app/styles/ilios-common/shared/main-list-box-header-title.scss

Copy link
Member

@stopfstedt stopfstedt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple of minor shifts, padding related i think, that the differ and Percy caught.
i'm ok with that.

@michaelchadwick michaelchadwick force-pushed the 6710-replace-font-size-sass-mixin branch from 229f6df to 42b17d0 Compare December 18, 2025 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run ui tests Run the expensive UI tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

font-size($step)

2 participants