Skip to content
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

Correctif pour les polices et icônes dsfr #4220

Merged
merged 7 commits into from Apr 16, 2024

Conversation

victormours
Copy link
Contributor

@victormours victormours commented Apr 15, 2024

Avant

Screenshot 2024-04-15 at 17 05 08 Screenshot 2024-04-15 at 17 05 44

Après

Screenshot 2024-04-15 at 17 05 19 Screenshot 2024-04-15 at 17 05 50

Le problème

Sur la page d'accueil de rdv mairie, on utilise les fiches de style minifiées du dsfr distribuées via le package node. Elles nécessitent que les polices soient disponibles via le path /fonts et les icônes via /icons.
A l'époque on avait résolu ça simplement en mettant en place des symlinks depuis public/fonts et public/icons vers ../node_modules/@gouvfr/dsfr/dist/fonts/ et ../node_modules/@gouvfr/dsfr/dist/icons/ (voir #3551, l'avantage étant que ça permettait d'éviter de committer les fonts et les icônes)

Or, depuis #4168, on n'inclus plus le dossier node_modules dans notre slug qui est déployé dans les containers scalingo, donc le symllink ne fonctionne pas, donc les polices ne se chargent pas (et on fallback sur Arial).

La solution

Pour rappel, quand on lance rails asset:precompile, ça lance un yarn run build, qui lance un webpack --config webpack.config.js, qui ensuite fait de la magie noire pour que les assets soient disponible dans public/assets/ et que les stylesheet_link_tag et javascript_include_tag pointent vers les bon path.

J'ai essayé un premier fix avec un plugin webpack (https://webpack.js.org/plugins/copy-webpack-plugin/), mais ça permettait uniquement de copier les fonts et icônes vers le path des assets webpack (avec la clé de cache webpack dans le path), et pas vers les path statiques /fonts et /icons.

Du coup je suis revenu vers un fix beaucoup plus simple : ajouter un bête cp unix équivalent aux symlinks.

Un fix plus durable

Pour limiter la différence entre la production et notre CI, je serais tenté de simuler la construction du slug scalingo en ajoutant une étape cat .slugignore | xargs rm -r à notre build de CI. En l'occurrence, ça aurait permis de voir cette erreur au moment du build et pas en production.
(Et contrairement à ce qu'on espérait dans la discussion mattermost, une gem de diff de screenshot n'aurait pas attrappé cette erreur, parce qu'on a encore les node_modules dans la CI).
C'est un peu plus complexe, donc je préfère garder ça dans un deuxième temps (et en priorité corriger la prod)

@victormours victormours marked this pull request as draft April 15, 2024 15:55
@victormours victormours marked this pull request as ready for review April 15, 2024 16:06
@victormours victormours marked this pull request as draft April 15, 2024 16:10
@victormours victormours marked this pull request as ready for review April 16, 2024 09:49
Copy link
Contributor

@francois-ferrandis francois-ferrandis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merci beaucoup pour cette investigation détaillée. 🤩

En effet le fix plus durable est à discuter dans un second temps. 👍

@victormours victormours enabled auto-merge (squash) April 16, 2024 14:58
@victormours victormours merged commit 874c4dd into production Apr 16, 2024
11 of 12 checks passed
@victormours victormours deleted the fix-dsfr-fonts-and-icons branch April 16, 2024 15:02
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.

None yet

2 participants