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

JS: migration à data-emplois- pour notre "setter" #5603

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xavfernandez
Copy link
Contributor

🤔 Pourquoi ?

Pour plus facilement identifier qui fournit le mécanisme utilisé.

🍰 Comment ?

Décrivez en quelques mots la solution retenue et mise en oeuvre, les difficultés ou problèmes rencontrés. Attirez l'attention sur les décisions d'architecture ou de conception importantes.

🚨 À 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

@xavfernandez xavfernandez added the no-changelog Ne doit pas figurer dans le journal des changements. label Feb 12, 2025
@xavfernandez xavfernandez self-assigned this Feb 12, 2025
@xavfernandez xavfernandez marked this pull request as ready for review February 12, 2025 15:37
@xavfernandez xavfernandez changed the title JS: migration à data-emplois- pour notre "setter" JS: migration à data-emplois- pour notre "setter" Feb 12, 2025
@xavfernandez xavfernandez changed the title JS: migration à data-emplois- pour notre "setter" JS: migration à data-emplois- pour notre "setter" Feb 12, 2025
Copy link
Contributor

@hellodeloo hellodeloo left a comment

Choose a reason for hiding this comment

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

👍

@xavfernandez xavfernandez force-pushed the xfernandez/prefix_data_attribute branch 2 times, most recently from 3fb6aca to 983714b Compare February 12, 2025 17:40
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.

Chouette d’avoir pensé à documenter ! 👏

migrations.md Outdated
Comment on lines 41 to 45

## Préfixer les attributs utilisé par notre JS par data-emplois

`data-bs-` est le préfixe utilisé par bootstrap, `data-it-` celui utilisé par le thème itou.
Notre JS devrait utiliser `data-emplois-`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Je pense que cette doc aurait plus sa place dans docs/javascript.md?
Quitte à garder une ligne ici pour dire de se conformer aux bonnes pratiques JS qui pointe vers docs/javascript?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On est d'accord que le docs/javascript.md n'existe pas ? 😅
L'idée serait donc d'en démarrer un ? Avec juste cette règle ?

Also remove the button restriction: it will soon be used from a span
(with role="button").
@xavfernandez xavfernandez force-pushed the xfernandez/prefix_data_attribute branch from 983714b to 788c2cc Compare February 14, 2025 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Ne doit pas figurer dans le journal des changements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants