Please include images in this project

[xiaoxiangmoe]

form-validation.scss include too many background-image with absolute path which works well in github

For example:

• background-image: url("/images/spinners/octocat-spinner-16px.gif");
• background: url("/images/spinners/octocat-spinner-32-EAF2F5.gif") 0 0 no-repeat;

@broccolini @shawnbot

if I use web application bundler like Parcel.js, it will crash for no file exists in /images/spinners/ dir in my computer.

[broccolini]

Thanks for letting us know about this issue @zhaojinxiang! We'll update this in our next release.

[shawnbot]

One way that we could handle this is to have one or more variables where the URLs of certain images live, so that non-github — or even just uses outside of github.com, such as in the style guide — could change them as needed.

[github-actions[bot]]

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

[xiaoxiangmoe]

@yaili Can we reopen this issue?

[dumptyd]

I'm running into a similar issue when I import S/CSS files from @primer/css and the webpack config is using css-loader>=4. Tracked it down to this: https://stackoverflow.com/a/63224177.

It would be good if it's resolved in @primer/css itself, but until its not, webpack can be configured this way to ignore these patterns:

resolve: {
  alias: {
    '/images/spinners': false,
    '/images/modules': false
  }
}

[ahmadnassri]

+1 on this, surprised this has been open since 2018!

[ChildishGiant]

We'll update this in our next release.

There have been 8 major releases since this and the issue is still prevalent :grimacing:

[github-actions[bot]]

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

[xiaoxiangmoe]

👀

[amirmasoudabdol]

Is there any updates on this?

[danielnc]

I am also having the same issue here

[deetter]

GOOD DAY/EVENING

[heynan0]

I'm trying to integrate the sass version of @primer/css into my team's application Rails asset pipeline, but this one is proving to be a problem. Our config.assets.prefix is /assets while @primer/css has hard-coded /images

This might be useful information, esp. for those using sass-loader in webpack:

https://github.com/webpack-contrib/sass-loader/tree/75dcfde159fe5508861f36ca577f71630c0d623e#problems-with-url

What's the update on the issue?

[howdyray]

@heynan0 Can you give us some more context to your use-case? We generally advise teams towards using PVC components instead of CSS clases to compose experiences. Would that help address your use case?

[heynan0]

@howdyray, it's not about using class names. We already use Primer View Components. The problem is integrating the .scss version of @primer/css into one's own asset pipeline.

Right now our team's application has to hard-code a CDN dist css version as a <link> tag. Upgrading the version in package.json does not automatically upgrade it since we also have to go to that template and update the URL in the <link> tag (point: someone might forget we have to do that and we cannot leverage dependabot version updates without manual intervention). That approach has been there since the introduction of dart sass functions in @primer/css that are not available in the now-obsolete sassc implementation that we have been using. We had no way of upgrading sass until now. And we want to be able to @use/import like this:

// application.scss (Rails)
@import "@primer/css/index.scss";

The above does not work properly because of lines such as:

https://github.com/primer/css/blob/55f35e1d0fc9a441f14f29b61ac5fbefbf3c4884/src/forms/form-validation.scss#L14

The above throws an error in the console. The path string passed to the url() function is hard-coded to start with /images/...

One's application might not have that exact path available.

In a default Rails application, the pipeline is served from /assets, not /images. The url would return 404. Therefore one would be forced to change the default asset prefix by setting config.assets.prefix to /images. Existing paths constructed with /assets would have to be changed else they would break. Also, not all assets are images necessarily; we could have icons and fonts, so the naming /images isn't truthful for one's own application assets prefix.

Another way would involve having to rewrite the path string prefixes to url(), something I could not figure out how to do.

As recommended in https://github.com/webpack-contrib/sass-loader/tree/75dcfde159fe5508861f36ca577f71630c0d623e#problems-with-url, the library could use a variable:

Library authors usually provide a variable to modify the asset path. bootstrap-sass for example has an $icon-font-path.

[simurai]

@heynan0 Thanks for the details. 🙇

We chatted about this today in our weekly meeting. Using a variable might be an option, but we think it's ok to just move these url('/images/... styles to dotocom so that Primer CSS doesn't reference any image paths. Reasoning:

• .is-autocheck-... classes get added/removed by the autocheck behavior that lives on dotcom.
• We already have other autocheck styles here that replaces the images with SVGs.
• These styles are kinda deprecated because we tried to replace all the gifs with the Spinner component in the past but just haven't finished the migration in all places.
• Same for status-indicator

I can make a PR with the removal.