Skip to content

feat: add <ContentMedia /> component (img, svg, iframe, video, audio) - #512

Open
habbichelotfi wants to merge 2 commits into
codegouvfr:mainfrom
habbichelotfi:feat/content-media-component
Open

feat: add <ContentMedia /> component (img, svg, iframe, video, audio)#512
habbichelotfi wants to merge 2 commits into
codegouvfr:mainfrom
habbichelotfi:feat/content-media-component

Conversation

@habbichelotfi

@habbichelotfi habbichelotfi commented Aug 28, 2026

Copy link
Copy Markdown

Closes #511

Contexte

Cette PR implémente le composant ContentMedia conformément à la structure DSFR pour l’intégration de médias accessibles et responsives.

Ce qui est inclus

  • ajout du composant ContentMedia
  • support des images
  • support de la légende (caption)
  • support du lien de légende (captionLinkProps)
  • documentation Storybook
  • typage TypeScript complet des props

Détails d’implémentation

  • composant basé sur la structure DSFR figure / figcaption
  • props typées pour guider l’usage et éviter les combinaisons invalides
  • exemples Storybook pour montrer les cas d’usage principaux
  • mise à jour des exemples de liens externes avec attributs de sécurité (target="_blank" + rel="noopener noreferrer")

Vérifications

  • TypeScript: pas d’erreurs sur les fichiers modifiés
  • Storybook: stories ContentMedia ajoutées et documentées
  • Accessibilité: structure sémantique DSFR respectée

@kevbarns

Copy link
Copy Markdown
Collaborator

@habbichelotfi Merci pour cette PR. Le découpage des props par type discriminé est le bon choix, et les alternatives <p> dans <video> / <audio> sont conformes à la doc DSFR (src/dsfr/component/content/_part/doc/code/index.md).

Vérifié en local sur ee6579b : yarn build, tsc -p src, prettier --list-different et yarn test (103/103) passent. La CI GitHub n'a en revanche jamais tourné : le run est en action_required (PR de fork, approbation d'un mainteneur requise).

Bloquant

ContentMedia n'est pas déclaré dans src/bin/only-include-used-components.ts. resolveModuleIdToDsfrComponents({ moduleId: "ContentMedia" }) renvoie undefined (vérifié à l'exécution), ce qui déclenche le fail-safe : dès qu'un projet importe le composant, le trimming CSS est désactivé pour tout le run et un console.warn est émis. Il faut ajouter "ContentMedia": ["content", "link"] à REACT_DSFR_MODULE_TO_DSFR_COMPONENTS — la doc DSFR liste Content (obligatoire) et Link (si lien dans la légende).

Le lien de légende peut être rendu sans nom accessible. captionLinkProps et captionLinkLabel sont indépendants : passer le premier sans le second produit <a class="fr-link"></a>. Détail en inline.

À discuter

SVG non enveloppé dans fr-content-media__img. La doc DSFR écrit « Un élément <div> de classe fr-content-media__img contient l'image. L'image est un élément <img> avec la classe fr-responsive-img, ou un <svg> », et example/component/content/index.html enveloppe bien le SVG — seul le snippet de la doc ne le fait pas. Ce wrapper porte width: 100% et sert d'ancrage aux variantes de taille.

Variantes de taille et ratios absents : pas de prop pour fr-content-media--sm / --lg, et les fr-ratio-* ne passent que par imgProps.className, ce qui n'est documenté nulle part.

Optionnel

Le diff modifie HighlightExample dans test/integration/vite/src/Home.tsx (apostrophes typographiques remplacées par des apostrophes droites), sans rapport avec le composant.

Rien à signaler côté sécurité.

Comment thread src/ContentMedia.tsx
Comment on lines +23 to +26
/** Text of the optional link inside the figcaption. */
captionLinkLabel?: ReactNode;
/** Props of the optional link inside the figcaption. */
captionLinkProps?: RegisteredLinkProps;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ces deux props sont indépendantes alors qu'elles n'ont de sens qu'ensemble : captionLinkProps sans captionLinkLabel rend <a class="fr-link"></a>, un lien sans nom accessible (RGAA 6.1, WCAG 2.4.4). Le composant n'étant pas encore publié, l'appariement peut se faire au niveau du type sans coût de migration.

Suggested change
/** Text of the optional link inside the figcaption. */
captionLinkLabel?: ReactNode;
/** Props of the optional link inside the figcaption. */
captionLinkProps?: RegisteredLinkProps;
/** Label and props of the optional link inside the figcaption. Both go together. */
captionLink?: {
label: ReactNode;
linkProps: RegisteredLinkProps;
};

La suggestion ne peut pas porter le site de rendu : il faut aussi remplacer les lignes 178-181 par un test sur captionLink et adapter les stories ainsi que test/integration/vite/src/Home.tsx.

A module absent from REACT_DSFR_MODULE_TO_DSFR_COMPONENTS resolves to
undefined, which trips the include-everything fail-safe: the script warns,
exits 0, and trims no CSS at all for any project importing it.

content for the fr-content-media markup, link for the fr-link the caption
renders.
@kevbarns

Copy link
Copy Markdown
Collaborator

Heads-up on something coming your way, @habbichelotfi. Nice component, the coverage of the five media types is thorough.

#515 adds an exhaustiveness test over REACT_DSFR_MODULE_TO_DSFR_COMPONENTS in only-include-used-components. Any module under src/ missing from that map resolves to undefined, which trips an include-everything fail-safe: the script warns, exits 0, and trims no CSS at all for every project importing it. ContentMedia is the case that surfaced the gap, so once #515 lands this PR would have failed CI until the map got an entry.

I pushed that entry to your branch in 9b75139 rather than leave you with a red CI:

"ContentMedia": ["content", "link"],

link is there because the caption renders fr-link via <Link className={fr.cx("fr-link")}>. The spacing utilities in the stories come from core, which is always included, so they need nothing. Verified by cherry-picking the #515 tests onto your branch: they fail naming ContentMedia without the entry and pass with it. Revert it if you would rather handle it yourself.

One related change worth knowing about: #515 also drops fr-responsive-img and fr-responsive-vid from the detection prefixes for content, since their base rules live in core and every content.css rule using them is scoped under .fr-content-media. That does not affect this component, which renders fr-content-media itself.

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.

Implement ContentMedia component

3 participants