Skip to content

feat: 💄 Wrap dsfr css imports in a dsfr layer #400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 6, 2025

Conversation

kaaloo
Copy link
Contributor

@kaaloo kaaloo commented Mar 31, 2025

This PR just wraps the included dsfr css in a dsfr layer. This allows codebases that are using the nextjs pages dir to better prioritize the dsfr css with respect to their own or third party libraries.

For example, at Réfugiés.info, we have a legacy codebase with several different frameworks historically including for example some bootstrap. We use a @layers directive to help prioritize these different and sometimes conflicting css files.

https://github.com/refugies-info/karfur/blob/89b999a95d7d547a3f0749da41c7e17a0d7379f7/apps/client/src/css/index.css#L1

Since react-dsfr imports dsfr.min.css in the Next Pages Router integration:

import "./assets/dsfr_plus_icons.css";

We currently resort to patching the dsfr.min.css file, however this requires updating the patch for each new react-dsfr release, which is not ideal.

https://github.com/refugies-info/karfur/blob/89b999a95d7d547a3f0749da41c7e17a0d7379f7/package.json#L26

@enguerranws
Copy link
Collaborator

Seems fine by me, just wondering if @layer breaks some compatibility with browsers / version that are currently supported by the DSFR?

@ddecrulle
Copy link
Collaborator

Seems fine by me, just wondering if @layer breaks some compatibility with browsers / version that are currently supported by the DSFR?

Same here, it looks good, but I can’t determine the impact this change might have on users of the library.

@garronej
Copy link
Collaborator

garronej commented Apr 1, 2025

Hello @kaaloo,

Thanks! I have no issue merging this PR, dsfr_plus_icons.css is only used with the Next.js Pages Router.
If you say it works, I believe you.

That said, could you please undo the changes you made to the .scss version of the file?
I have zero trust in Next.js not to break unexpectedly from seemingly insignificant changes.

The fact that we have to rely on SCSS just because the App Router’s CSS compiler can’t guarantee deterministic stylesheet order is honestly baffling to me.

Anyway, once you revert the SCSS changes, I’ll go ahead and merge + release.

@kaaloo
Copy link
Contributor Author

kaaloo commented Apr 3, 2025

Hi @garronej , sorry I was just able to get back to this. I reverted my changes to the .scss file just now. Thanks for your help !

@garronej
Copy link
Collaborator

garronej commented Apr 6, 2025

Thank you :)

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

Successfully merging this pull request may close these issues.

4 participants