Skip to content

Nouveautés: correction de l'affichage de la modale#6313

Merged
xavfernandez merged 2 commits into
masterfrom
xf/supports_local_storage
Jun 12, 2025
Merged

Nouveautés: correction de l'affichage de la modale#6313
xavfernandez merged 2 commits into
masterfrom
xf/supports_local_storage

Conversation

@xavfernandez

@xavfernandez xavfernandez commented Jun 11, 2025

Copy link
Copy Markdown
Contributor

🤔 Pourquoi ?

La fonction supports_local_storage n'était dispo que sur la démo & les recettes jetables, pour l'affichage de la modale de connexion des comptes de démo.

La modale de nouveautés n'a donc jamais fonctionné en production 🙈 🥲 .

🍰 Comment ?

En définissant la fonction uniquement là où elle est utilisée.

Mais je me demande si on ne pourrait pas complètement la supprimer au vu de la compatibilité des navigateurs:
https://developer.mozilla.org/en-US/docs/Web/API/Window/localStorage#browser_compatibility

🚨 À vérifier

  • Mettre à jour le CHANGELOG_breaking_changes.md ?
  • Ajouter l'étiquette « Bug » ?

🏝️ Comment tester ?

Les instructions pour reproduire le problème, les profils de test, le parcours spécifique à utiliser, etc. Si vous disposez d'une recette jetable, mettre l'URL pour tester dans cette partie.

💻 Captures d'écran

supports_local_storage was only available in demo & review-apps as a
side-effect of the demo_accounts modal.

Instead define the helper where it is needed.
@xavfernandez xavfernandez self-assigned this Jun 11, 2025
@xavfernandez xavfernandez added modifié Modifié dans le changelog. bug labels Jun 11, 2025
@xavfernandez xavfernandez requested a review from rsebille June 11, 2025 12:47

@rsebille rsebille left a comment

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.

Effectivement on pourrais se dire qu'on la supprime : https://caniuse.com/mdn-api_window_localstorage
Ou alors la mettre dans utils.js ou un compat.js chargé plus haut 🤷, au moins ça évitera la duplication de code.

local_storage is widely available in all major browsers since at least
10 years
@xavfernandez

Copy link
Copy Markdown
Contributor Author

J'ai droppé la fonction. Je reste sur 2 commits pour que le problème et son fix soient bien clairs.

@xavfernandez xavfernandez added this pull request to the merge queue Jun 12, 2025
Merged via the queue into master with commit 51850d5 Jun 12, 2025
14 checks passed
@xavfernandez xavfernandez deleted the xf/supports_local_storage branch June 12, 2025 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug modifié Modifié dans le changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants