feat(BackToTop): add the BackToTop component - #516
Open
kevbarns wants to merge 2 commits into
Open
Conversation
Picks up the work started in #139 by @sylvainlg, who confirmed it could be taken over, and addresses the review left open there. The default is the DSFR canonical form: an `<a href="#top">` carrying `fr-link`, the `arrow-up-fill` icon on the left and the "Haut de page" label. The anchor is what moves the navigation focus back to the top of the page, not just the scroll, so it stays the default rather than a scripted scroll. For pages that cannot set `id="top"` on their topmost element, `targetRef` renders a `<button type="button">` styled as a link, which scrolls the element into view, moves the focus onto it and honours `prefers-reduced-motion`. `anchor` and `targetRef` are mutually exclusive at the type level. Registers the component in REACT_DSFR_MODULE_TO_DSFR_COMPONENTS so the component CSS optimizer keeps trimming when a consumer imports it.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new BackToTop component to the React DSFR component set, aligned with DSFR guidance and the prior discussion in #139 (including an accessible targetRef variant that restores focus when scrolling programmatically).
Changes:
- Added
BackToTopcomponent with two mutually exclusive APIs:anchor(default#top) ortargetRef(renders a<button>and performs scroll + focus). - Added Storybook stories and documentation for the component and its prop semantics.
- Updated the CSS optimizer mapping so importing
BackToTopresolves to the DSFRlinkCSS component.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/BackToTop.tsx |
Implements the new BackToTop component (anchor default + accessible targetRef behavior) and its i18n strings. |
stories/BackToTop.stories.tsx |
Adds Storybook documentation/stories and prop typing assertions for BackToTop. |
src/bin/only-include-css-of-used-components.ts |
Ensures CSS optimizer includes DSFR link styles when BackToTop is used. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…scroll `behavior: "auto"` does not mean "do not animate", it means "use the CSS scroll-behavior". On a page that sets `scroll-behavior: smooth`, the prefers-reduced-motion branch animated the scroll anyway, so it did nothing. Measured in a browser by reading scrollTop synchronously right after the call, on a container whose stylesheet sets `scroll-behavior: smooth`: `behavior:"auto"` leaves scrollTop untouched (animated), `"instant"` lands it immediately. `"instant"` is not the fix: it is absent from TypeScript 4.9's ScrollBehavior and throws a TypeError on browsers that predate it, which would break the handler after the focus had already moved. The CSS is neutralised inline and restored instead, which is what the DSFR does around its own scroll lock. Also stop leaving a tabindex behind on the consumer's node, drop the story that pointed at an anchor absent from the Storybook canvas, and drop the keyof assert that had no precedent in stories/.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reprise de #139, ouverte par @sylvainlg en juin 2023. Il a confirmé le 31/08 que le travail pouvait être repris (#139 (comment)). La structure, le nommage des props et les stories viennent de sa PR.
Closes #139
Ce que la revue de #139 laissait ouvert
La seule remarque non traitée était #139 (comment), que @garronej avait marquée comme bloquante. Elle demandait trois choses :
<button>styléfr-link, parce qu'un<a>sanshrefvalide n'est pas accessible ;refplutôt qu'un sélecteur CSS pour désigner la cible, « Dsfr components do not allow to provide html id but all enable to provide ref ».Les points 2 et 3 sont implémentés tels quels. Le point 1 est implémenté mais pas en défaut, et c'est la seule divergence assumée de cette PR.
API
anchorettargetRefsont mutuellement exclusives au niveau du type.Pourquoi l'ancre reste le défaut
La doc DSFR est explicite sur ce composant (
src/dsfr/component/link/example/back-to-top/index.ejsdans le paquet@gouvfr/dsfr) :C'est le point qui tranche : un
window.scrollTodéplace le viewport mais pas le focus. Vérifié dans un navigateur, sur une page avec<body id="top">: le focus posé sur un lien en milieu de page, un clic sur<a href="#top">fait passerdocument.activeElementde ce lien àbody#top. Un scroll scripté ne produit pas ce déplacement.La variante
targetRefcouvre donc le besoin de #139 sans ce défaut : elle faitscrollIntoViewetfocus({ preventScroll: true })sur la cible, en posanttabindex="-1"si l'élément n'est pas déjà atteignable.@garronej si tu préfères malgré tout
targetRefen défaut, ou une propsmoothsupplémentaire, dis-le et je change.prefers-reduced-motion
La première version de cette PR passait
behavior: "auto"quandprefers-reduced-motion: reduceest actif. C'était faux et la description l'annonçait à tort comme réglé."auto"ne veut pas dire « ne pas animer », il veut dire « prends la valeur CSS descroll-behavior» : sur une page qui posescroll-behavior: smooth, l'animation avait lieu quand même.Mesuré en lisant
scrollTopde façon synchrone juste après l'appel, sur un conteneur dont la feuille de style posescroll-behavior: smooth— un scroll instantané a déjà atterri, un scroll animé non :behaviorpasséscroll-behaviorCSSscrollTopavant → après"auto"smooth"instant"smooth"smooth"smooth"auto"auto"instant"n'est pas le correctif pour autant : il est absent deScrollBehaviordans lelib.dom.d.tsde TypeScript 4.9, la version du dépôt, et une valeur d'enum WebIDL inconnue lève unTypeError, ce qui casserait le handler après que le focus a déjà bougé. Le CSS est donc neutralisé en inline puis restauré, ce que fait déjà le DSFR autour de son propre scroll lock (dsfr.module.js,ScrollLock.lock). Corrigé en 125a422.Rendu vérifié
Markup obtenu via
renderToStaticMarkup, à comparer au sample DSFRlink-back-to-top.ejs(label: 'Haut de page',href: '#top',icon: 'arrow-up-fill',iconPlace: 'left') :Optimiseur CSS
"BackToTop": ["link"]est ajouté àREACT_DSFR_MODULE_TO_DSFR_COMPONENTS. Vérifié de bout en bout avecnpx react-dsfr optimize-csssur un projet jetable qui n'importe queBackToTop:dsfr.min.csstombe à 200 664 octets,fr-linkconservé,fr-tableetfr-accordionabsents. Sans l'entrée dans la map, le même projet donne :soit 591 050 octets. L'entrée vaut donc 390 ko de CSS sur ce projet, et son absence est bien le fail-safe. Le test
publicModuleCoverageéchoue également en nommantBackToTop (from src/BackToTop): resolves to undefined.L'icône n'est pas rognée non plus :
npm pack @codegouvfr/react-dsfr@1.33.0montre que le paquet publié embarquesrc/(386 fichiers), et le scanner d'icônes descend dansnode_modules/@codegouvfr/react-dsfr/src, où le littéralfr-icon-arrow-up-fillest présent.Vérifications lancées
yarn buildtsc -p src --noEmittsc -p src/bin --noEmitsrc/tsconfig.jsonexclut./bin, donc couvert séparément)yarn testyarn format:checkeslintsur les fichiers touchésnpx react-dsfr optimize-csssur projet jetableLimites
tabindexau blur n'est pas vérifié. Le composant posetabindex="-1"sur la cible si elle n'est pas déjà atteignable, et le retire surblur. Retirer l'attribut juste aprèsfocus()ne marche pas, c'est mesuré : l'élément est blurré etdocument.activeElementretombe sur<body>. Leblurlui-même n'a pas pu être testé, l'environnement de test ne dispatche aucun événement de focus (document.hasFocus()estfalse). Si le blur ne vient jamais, ce qui reste est untabindex="-1", qui par définition garde l'élément hors de l'ordre de tabulation : le mode dégradé est inerte.targetRef:getStoryne passe que des props sérialisables au composant, donc la variante est documentée par un bloc de code dans la description, comme le fait déjàAccordionpour son mode contrôlé.jsdomni@testing-library/react). Le markup ci-dessus a été vérifié viareact-dom/serverdans un fichier jetable, etscrollBackTodans un navigateur en injectant la fonction telle que compilée dansdist/BackToTop.js.