Skip to content

Build icons as plain SVG - #519

Open
firestar300 wants to merge 2 commits into
masterfrom
ci/svg
Open

firestar300 wants to merge 2 commits into
masterfrom
ci/svg

Conversation

@firestar300

Copy link
Copy Markdown
Contributor

Summary

Cette PR enrichit la chaîne de build et l’affichage des icônes SVG du thème :

  • Build : en plus des sprites dans dist/icons/, chaque SVG source sous src/img/icons/ est optimisé (SVGO) et copié dans dist/images/ en conservant l’arborescence (sprite/, social/, etc.).
  • PHP : get_the_icon() / the_icon() acceptent un 3ᵉ paramètre $is_sprite (défaut true) pour choisir entre référence sprite (<use>) et SVG fichier inline.
  • Config SVGO : alignement sur SVGO 4 (suppression de l’override removeViewBox obsolète qui provoquait un warning au build).

Le comportement existant (sprites + cache-busting via sprite-hashes.asset.php) reste inchangé par défaut.

Changements techniques

Zone Détail
config/webpack-icon-files-plugin.js Nouveau plugin Webpack (afterEmit) : lecture src/img/icons/**/*.svg → écriture dist/images/** avec config/svgo.config.js
config/plugins.js Enregistrement du plugin
config/svgo.config.js Preset SVGO 4 simplifié
package.json Dépendance svgo (utilisée par le plugin)
inc/Services/Svg.php $is_sprite, parsing d’identifiant, chargement sécurisé du fichier, injection des classes sur la balise <svg> racine
inc/Helpers/Svg.php Propagation du paramètre $is_sprite

Exemples d’usage

// Sprite (comportement actuel)
\BEA\Theme\Framework\Helpers\Svg\the_icon( 'menu' );
\BEA\Theme\Framework\Helpers\Svg\get_the_icon( 'social/facebook' );

// SVG fichier complet (inline)
\BEA\Theme\Framework\Helpers\Svg\the_icon( 'menu', [], false );
\BEA\Theme\Framework\Helpers\Svg\get_the_icon( 'social/facebook', [ 'post-sharing__icon' ], false );

Chemins générés côté build, par exemple :

  • src/img/icons/sprite/menu.svg → dist/icons/sprite.svg (sprite) et dist/images/sprite/menu.svg (fichier)
  • src/img/icons/social/facebook.svg → dist/icons/social.svg et dist/images/social/facebook.svg

Test plan

  • npm run build sans erreur (vérifier l’absence du warning SVGO removeViewBox)
  • Présence des sprites : dist/icons/sprite.svg, dist/icons/social.svg
  • Présence des SVG unitaires : dist/images/sprite/*.svg, dist/images/social/*.svg
  • Front : icônes existantes en sprite (the_icon( 'share' ), partage d’article, etc.) inchangées visuellement
  • Test manuel du mode inline : the_icon( 'menu', [], false ) — markup SVG complet dans le HTML, classes icon / icon-menu présentes
  • Test identifiant réseau social : get_the_icon( 'social/facebook', [], false ) — fichier dist/images/social/facebook.svg bien servi
  • Cas limite : identifiant inconnu ou fichier manquant → chaîne vide, pas d’erreur PHP

Notes

  • Les SVG inline proviennent uniquement de dist/images/ (artefacts de build du thème) ; le chemin est validé via realpath sous dist/images.
  • Pour l’optimisation SVG, la stack du thème s’appuie sur SVGO (svgo-loader pour les sprites, plugin dédié pour les fichiers unitaires, imagemin-svgo dans la minification d’images).

Comment thread config/svgo.config.js
Comment on lines -5 to -10
params: {
overrides: {
// Disable a plugin included by default that you don't want (false)
removeViewBox: false,
},
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

C'est volontaire ?

Sur le svg généré, il n'y aura plus de viewbox. De mémoire ça posait des soucis de taille.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Je suis passé à SVGO 4 sur cette PR : https://github.com/BeAPI/beapi-frontend-framework/pull/518/changes

Et avait justement remanié le fichier pour éviter le warning, car la structure change. Il faut le mettre dans le tableau en dessous

Comment thread inc/Helpers/Svg.php
* @return string
*/
function get_the_icon( string $icon_class, $additionnal_classes = [] ): string {
function get_the_icon( string $icon_class, $additionnal_classes = [], bool $is_sprite = true ): string {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On part du principe qu'on utilise toujours des sprite par défaut ? pas l'inverse ?

Comment thread inc/Services/Svg.php
* @return string
*/
public function get_the_icon( string $icon_class, array $additionnal_classes = [] ): string {
public function get_the_icon( string $icon_class, array $additionnal_classes = [], bool $is_sprite = true ): string {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Même remarque

Comment thread package.json
"stylelint-scss": "^6.14.0",
"stylelint-webpack-plugin": "^5.1.0",
"svg-sprite-loader": "^6.0.11",
"svgo": "^4.0.2",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

#518 j'avais migé ici déjà, est-ce que ça vaut le coup de merger cette PR et rebase dans ta branche ?

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.

2 participants