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

UX/UI: Problème de rechargement des sliding-tabs apres reload htmx #5023

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

hellodeloo
Copy link
Contributor

@hellodeloo hellodeloo commented Oct 31, 2024

🤔 Pourquoi ?

Les onglets data-it-sliding-tabs="true" n’est pas réinterprété en "sliding-tabs" après un rechargement partiel htmxx

@hellodeloo hellodeloo self-assigned this Oct 31, 2024
@hellodeloo hellodeloo changed the title fix: sliding tabs on htmx reload UX/UI: Problème de rechargement des sliding-tabs apres reload htmx Oct 31, 2024
itou/static/js/sliding_tabs.js Outdated Show resolved Hide resolved
Copy link
Contributor

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

Un patch avec suggestions d’amélioration :

  • Ne chercher des sliding tabs que dans target et non document, pour ne pas initialiser deux fois les onglets sur un document lors du chargement d’une réponse HTMX
  • Utilisation du sélecteur CSS précis au lieu du commentaire pas clair
  • Simplification du calcul de startIndex, et utilisation d’entiers et non d’une string (slidingTabs.getAttribute("data-it-sliding-tabs-startindex") === "2")
  • Préférer const à let, pour indiquer qu’il n’y aura pas de changements
diff --git a/itou/static/js/sliding_tabs.js b/itou/static/js/sliding_tabs.js
index 2825c1e15..04ce4cb9e 100644
--- a/itou/static/js/sliding_tabs.js
+++ b/itou/static/js/sliding_tabs.js
@@ -1,15 +1,10 @@
 "use strict";
 
-htmx.onLoad(function (target) {
-  // Only for <ul.s-tabs-01__nav.nav.nav-tabs> how has data-it-sliding-tabs="true" attribute
-  let slidingTabs = document.querySelector("[data-it-sliding-tabs=true]");
+htmx.onLoad(function(target) {
+  const slidingTabs = target.querySelector("ul.s-tabs-01__nav.nav.nav-tabs[data-it-sliding-tabs=true]");
 
   if (slidingTabs) {
-    let slidingTabsStartIndex = 0;
-
-    if (slidingTabs.hasAttribute("data-it-sliding-tabs-startindex")) {
-      slidingTabsStartIndex = slidingTabs.getAttribute("data-it-sliding-tabs-startindex")
-    }
+    const slidingTabsStartIndex = Number.parseInt(slidingTabs.getAttribute("data-it-sliding-tabs-startindex")) || 0;
 
     tns({
       container: slidingTabs,

@hellodeloo hellodeloo force-pushed the deloo/fix-sliding-tabs-on-htmx-reload branch from bab9e7e to c87f078 Compare November 4, 2024 10:09
@hellodeloo
Copy link
Contributor Author

@francoisfreitag Merci, je prends tout sauf le "sélecteur CSS précis" car trop précis.
J'ai une autre mise en forme de slider (pas les même classes) qui utilise aussi le slide via [data-it-sliding-tabs=true]

@francoisfreitag
Copy link
Contributor

On pourrait aller un poil plus loin avec un querySelectorAll.

@francoisfreitag
Copy link
Contributor

francoisfreitag commented Nov 4, 2024

"use strict";

htmx.onLoad(function(target) {
  target.querySelectorAll("[data-it-sliding-tabs=true]").forEach(function(slidingTabs) {
    const slidingTabsStartIndex = Number.parseInt(slidingTabs.getAttribute("data-it-sliding-tabs-startindex")) || 0;
    tns({
      container: slidingTabs,
      slideBy: "page",
      autoWidth: true,
      arrowKeys: true,
      loop: false,
      mouseDrag: true,
      swipeAngle: false,
      speed: 300,
      nav: false,
      controls: true,
      startIndex: slidingTabsStartIndex,
    });
  })
});

@francoisfreitag
Copy link
Contributor

Cette nouvelle version supporte plusieurs éléments sliding tabs dans une page (ou un fragment) et n’a pas besoin du if (slidingTabs) {}, ce qui me semble un peu plus clair.

@hellodeloo hellodeloo force-pushed the deloo/fix-sliding-tabs-on-htmx-reload branch from c87f078 to 94fbf9b Compare November 7, 2024 14:17
@hellodeloo
Copy link
Contributor Author

Cette nouvelle version supporte plusieurs éléments sliding tabs dans une page (ou un fragment) et n’a pas besoin du if (slidingTabs) {}, ce qui me semble un peu plus clair.

Quelle concision ✂️

@hellodeloo hellodeloo added this pull request to the merge queue Nov 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 7, 2024
@hellodeloo hellodeloo force-pushed the deloo/fix-sliding-tabs-on-htmx-reload branch from 94fbf9b to 803a1d1 Compare November 12, 2024 08:03
@hellodeloo hellodeloo added this pull request to the merge queue Nov 12, 2024
Merged via the queue into master with commit e238da3 Nov 12, 2024
9 checks passed
@hellodeloo hellodeloo deleted the deloo/fix-sliding-tabs-on-htmx-reload branch November 12, 2024 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants