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

754 les onglets ne sont plus sticky quand on change de route #755

Merged

Conversation

yaaax
Copy link
Collaborator

@yaaax yaaax commented Jul 5, 2024

No description provided.

@yaaax yaaax requested a review from bellangerq July 5, 2024 20:37
@yaaax yaaax linked an issue Jul 5, 2024 that may be closed by this pull request
@yaaax yaaax self-assigned this Jul 5, 2024
@yaaax yaaax added the bug Ça marche pas comme prévu label Jul 5, 2024
@yaaax yaaax added this to the Spring Summer 4 - 18/07 milestone Jul 5, 2024
@hissalht hissalht temporarily deployed to ara-754-les-onglets-ne--fw6hol July 5, 2024 20:41 Inactive
@yaaax yaaax removed this from the Spring Summer 4 - 18/07 milestone Jul 5, 2024
Comment on lines 160 to 171
onMounted(async () => {
const el = await waitForElement("#sticky-indicator");
const resizeObserver = new ResizeObserver((entries) => {
stickyTop.value = entries[0].target.clientHeight + "px";
// Because auditGenerationHeader ref is inside a "v-if",
// Vue will not instantiate the ref immediately.
// We need to watch it before observing nested stickyIndicator
watch(auditGenerationHeader, async () => {
const stickyIndicator = auditGenerationHeader.value?.stickyIndicator;
resizeObserver = new ResizeObserver((entries) => {
stickyTop.value = entries[0].target.clientHeight + "px";
});
stickyIndicator && resizeObserver.observe(stickyIndicator);
});
resizeObserver.observe(el);
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

On a le "droit" de mettre un watch dans un onMounted ?

Copy link
Collaborator Author

@yaaax yaaax Jul 11, 2024

Choose a reason for hiding this comment

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

Bien vu @hissalht . Je pense que c’est permis, mais clairement inutile et ça devait surement créer un memory leak.
Voir : https://vuejs.org/guide/essentials/watchers#stopping-a-watcher

J’ai simplement enlevé le onMounted pour ne laisser que le watch.
Voir 5856a49.

@AdrienMuzyczka
Copy link
Collaborator

Nickel pour l'ajustement entre le composant "ajout de fichier" et "l'impact usager". En revanche, j'ai noté deux petites choses (je ne sais pas à quoi c'est dû) :

  • Les libellés des champs sont en gras alors qu'ils devraient être en regular comme sur la prod.
  • Il y a un bug au scroll : les onglets sont bien sticky, mais lorsqu'on descend dans la page, le premier critère passe par-dessus les onglets :
    Capture d’écran 2024-07-11 à 10 07 48

@yaaax
Copy link
Collaborator Author

yaaax commented Jul 11, 2024

@AdrienMuzyczka :
Ici on ne traite que le problème des onglets qui n’étaient plus sticky quand on change de route, pas de modifs sur :

  • l’espace entre "ajout de fichier" et "l'impact usager" (l’espace plus petit est surement une régression introduite par un autre commit)
  • les libellés en gras : c'est normal, ça a été corrigé après que cette branche ait été tirée ;)
  • le "bug" du scroll n’est pas un bug, ça a toujours été comme ça quand le focus est dans la zone de texte "Erreurs et recommandations" ou "Commentaires" (tu verras, si tu enlèves le focus, ça revient à la normale)

@hissalht hissalht had a problem deploying to ara-754-les-onglets-ne--fw6hol July 11, 2024 08:45 Failure
@AdrienMuzyczka
Copy link
Collaborator

@AdrienMuzyczka : Ici on ne traite que le problème des onglets qui n’étaient plus sticky quand on change de route, pas de modifs sur :

  • l’espace entre "ajout de fichier" et "l'impact usager" (l’espace plus petit est surement une régression introduite par un autre commit)
  • les libellés en gras : c'est normal, ça a été corrigé après que cette branche ait été tirée ;)
  • le "bug" du scroll n’est pas un bug, ça a toujours été comme ça quand le focus est dans la zone de texte "Erreurs et recommandations" ou "Commentaires" (tu verras, si tu enlèves le focus, ça revient à la normale)

Ça marche :)

@yaaax yaaax force-pushed the 754-les-onglets-ne-sont-plus-sticky-quand-on-change-de-route branch from dc91341 to d0bd5af Compare July 11, 2024 09:19
yaaax added 3 commits July 11, 2024 11:31
* expose stickyIndicator from AuditGenerationHeader

* watch for ref updates in AuditGenerationPage

Note that nested stickyIndicator height is used with 2 sticky divs: filters and tabs
@yaaax yaaax force-pushed the 754-les-onglets-ne-sont-plus-sticky-quand-on-change-de-route branch from d0bd5af to 5c40cf8 Compare July 11, 2024 09:35
@hissalht hissalht temporarily deployed to ara-754-les-onglets-ne--fw6hol July 11, 2024 09:35 Inactive
@hissalht hissalht temporarily deployed to ara-754-les-onglets-ne--fw6hol July 11, 2024 13:54 Inactive
@yaaax yaaax force-pushed the 754-les-onglets-ne-sont-plus-sticky-quand-on-change-de-route branch from 8f8d04e to 5856a49 Compare July 11, 2024 13:57
@hissalht hissalht temporarily deployed to ara-754-les-onglets-ne--fw6hol July 11, 2024 13:57 Inactive
@yaaax yaaax merged commit 4e3ad20 into main Jul 11, 2024
1 check passed
@yaaax yaaax deleted the 754-les-onglets-ne-sont-plus-sticky-quand-on-change-de-route branch July 11, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Ça marche pas comme prévu
Projects
None yet
4 participants