sensei icon indicating copy to clipboard operation
sensei copied to clipboard

[Proposal] Use `senseiAssets` alias for relative assets in SCSS files

Open jom opened this issue 3 years ago • 1 comments

Changes proposed in this Pull Request

  • When importing Sensei SCSS assets into Sensei Pro, I'm running into issues where sometimes sass-loader isn't resolving relative paths correctly. I'm proposing we switch to using a named alias in the URLs that we can name in both Sensei and Sensei Pro's webpack.

  • If we merge, I'll add this alias to our Sensei Pro webpack.

  • I'd be open to just using a new sensei alias (to match what we have in Sensei Pro (then doing sensei/assets). I was quasi-wondering about conflicts, but that might not be an issue.

Testing instructions

  • Run npm run build
  • Ensure it builds without errors.
  • Check Sensei for missing styles.

jom avatar Oct 13 '22 17:10 jom

Codecov Report

Merging #5929 (568a153) into feature/new-onboarding-experience (1891e2e) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@                         Coverage Diff                          @@
##             feature/new-onboarding-experience    #5929   +/-   ##
====================================================================
  Coverage                                43.83%   43.83%           
  Complexity                                9143     9143           
====================================================================
  Files                                      439      439           
  Lines                                    32521    32521           
  Branches                                   252      252           
====================================================================
  Hits                                     14257    14257           
  Misses                                   18078    18078           
  Partials                                   186      186           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 541edc2...568a153. Read the comment docs.

codecov[bot] avatar Oct 13 '22 17:10 codecov[bot]

I definitely agree that it would be ideal to use relative paths. I worked with sass-loader configs for a while trying to get it to work, but would be super happy if someone knew the magic to get it working.

jom avatar Oct 14 '22 14:10 jom

@jom Do you have any ways to reproduce the issue properly? Maybe we could try to find a proper solution together.

I particularly don't remember seeing that issue before...

fjorgemota avatar Oct 14 '22 14:10 fjorgemota

@fjorgemota I sent it to you on Slack.

jom avatar Oct 14 '22 15:10 jom

Just for closing the loop, I sent PRs #5947 and 1819-gh-Automattic/sensei-pro to fix this issue using resolve-url-loader. :)

fjorgemota avatar Oct 14 '22 21:10 fjorgemota