Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

[webpack-config-terra] Replace node-sass with sass (dart-sass) #676

Closed
mjhenkes opened this issue Jun 22, 2021 · 1 comment · Fixed by #685
Closed

[webpack-config-terra] Replace node-sass with sass (dart-sass) #676

mjhenkes opened this issue Jun 22, 2021 · 1 comment · Fixed by #685

Comments

@mjhenkes
Copy link
Contributor

mjhenkes commented Jun 22, 2021

Feature Request

Description

As of October 2020 [node-sass]9https://www.npmjs.com/package/node-sass) is deprecated. The sass npm package is the replacement.

While the upgrade to this package is incredibly easy (see this branch), this change has a ripple effect on screenshot comparisons downstream.

Specifically the sass package calculates a greater precision than the node-sass package.

For example, the sass package has a precision of 10 which is non adjustable. See line-height in terra-base.

body {
    color: #1c1f21;
    color: var(--terra-base-color, #1c1f21);
    font-size: 1rem;
    height: 100%;
    height: var(--terra-base-body-height, 100%);
    line-height: 1.4285714286;
    line-height: var(--terra-base-line-height, 1.4285714286)
}

Where as the node-sass package has a default precision of 5

body {
    color: #1c1f21;
    color: var(--terra-base-color, #1c1f21);
    font-size: 1rem;
    height: 100%;
    height: var(--terra-base-body-height, 100%);
    line-height: 1.42857;
    line-height: var(--terra-base-line-height, 1.42857)
}

The effect in this specific instance on wdio tests is that content down the page increasingly gets 'off' according to the screenshot comparison:
open_fullscreen_modal

Options

  1. Consider this passive and require consuming teams and ourselves to update screenshots.
  2. Consider this passive, hunt down any css variable with greater than 5 precision and update it prior to release.
  3. Consider this non passive and include it in the next mvb, or release an mvb.

Additional Context / Screenshots

@ Mentions

@mjhenkes
Copy link
Contributor Author

An update here, we should treat this as a major version bump, option 3 of the options above. We shouldn't change css variables an instead document that teams should regenerate screenshots.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants