Switch to dart-sass #3919
Switch to dart-sass #3919
Conversation
sowasred2012
added
Review: QA needed
Review: Code needed
Feature 馃巵
sowasred2012
force-pushed
the
dart-sass
branch
from
f7c0ddc
to
7c9399e
Compare
|
@sowasred2012 The |
sowasred2012
force-pushed
the
dart-sass
branch
from
7a80515
to
8d207da
Compare
sowasred2012
force-pushed
the
dart-sass
branch
from
8d207da
to
f99827e
Compare
Co-authored-by: Bartek Szopka <83575+bartaz@users.noreply.github.com>
Co-authored-by: Bartek Szopka <83575+bartaz@users.noreply.github.com>
Co-authored-by: Bartek Szopka <83575+bartaz@users.noreply.github.com>
Co-authored-by: Bartek Szopka <83575+bartaz@users.noreply.github.com>
| @@ -3,14 +3,13 @@ | |||
| @mixin vf-b-typography-definitions { | |||
| //@section Heading styling in placeholders | |||
| %vf-heading-1 { | |||
| @include heading-max-width--short; | |||
There was a problem hiding this comment.
Is this related to dart specifically? or just a drive by?
There was a problem hiding this comment.
Just a drive by - I'll make a note
| // update the fallback with major releases | ||
| $app-version: '2.0.0' !default; | ||
| } | ||
| $app-version: '2.0.0' !default; |
There was a problem hiding this comment.
It's a 3.0 branch, so why reverting to 2.0?
There was a problem hiding this comment.
In an initial commit, we switched it to 3.0, then found that a number of things were broken - this also explains the heading-max-width--short things you noticed, we made a start on them, then realised it was going to be a bigger task (which we already have here #3744), and outside the scope of this PR, which was just to switch to dart-sass.
| <li class="p-navigation__link{% if path == '/accessibility' %} is-selected{% endif %}"><a href="/accessibility">Accessibility</a></li> | ||
| <li class="p-navigation__link{% if path == '/browser-support' %} is-selected{% endif %}"><a href="/browser-support">Browser support</a></li> | ||
| <li class="p-navigation__link{% if path == '/contribute' %} is-selected{% endif %}"><a href="/contribute">Contribute</a></li> | ||
| <li class="p-navigation__item{% if path == '/accessibility' %} is-selected{% endif %}"><a class="p-navigation__link" href="/accessibility">Accessibility</a></li> |
There was a problem hiding this comment.
Also I'm not sure why this is in this PR. It's fine if drive-by, but lets list it in the description (and link to fixed issues if we have them).
There was a problem hiding this comment.
There wasn't an issue for this, it was spotted when we initially switched the version to 3.0 and some deprecated styles were excluded. In any case, it's old markup that should have been updated, so I'll list it as an item in the description
There was a problem hiding this comment.
LGTM thanks
@sowasred2012 Knowing that removing deprecated stuff should we maybe revisit the estimate on the "remove deprecated" task?
Co-authored-by: Steve Rydz steve.rydz@canonical.com
Done
node-sassforsasspackage, updates SCSS build scripts accordingly.node-sassbuild information from the docssassdoesn't support passing a JS file with custom functions, open to suggestions on ways around this. For now, the version number is hardcoded.Drive bys
This being the first PR targeting the
vanilla-3.0branch, we initially updated the Vanilla version to 3.0 everywhere as well as switching todart-sass. We fixed some things that were broken by the exclusion of deprecated styles before realising we were biting off more than we could chew in the scope of this PR (and that these changes should be addressed in #3744), so reverted the version number to 2.0, and kept the changes we'd made:heading-max-width--shortet al mixins with the suggestedmap-get($max-widths, default)Fixes #3900 & #3756
QA
Check if PR is ready for release
If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:
Feature 馃巵,Breaking Change 馃挘,Bug 馃悰,Documentation 馃摑,Maintenance 馃敤.package.jsonshould be updated relative to the most recent release, following semver convention: