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

[BUGFIX] Placer les banières d'alerte dans une balise header #846

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

theotime2005
Copy link
Contributor

🎄 Problème

Les PixBannerAlert affiche un message mais ne sont pas placées dans des balise de banière tels que:

<header></header>

🎁 Proposition

Remplacer la balise div par une balise header.

🌟 Remarques

On pourrait garder le div et rajouter le header.

🎅 Pour tester

Les instructions pour reproduire le problème, les profils de test, le parcours spécifique à utiliser, etc.

@theotime2005 theotime2005 self-assigned this Mar 19, 2025
@theotime2005 theotime2005 force-pushed the place-banner-in-header branch from f5004cf to 53552b8 Compare March 19, 2025 09:52
@pix-bot-github
Copy link

Une fois l'application déployée, elle sera accessible à cette adresse https://ui-pr846.review.pix.fr
Les variables d'environnement seront accessibles sur scalingo https://dashboard.scalingo.com/apps/osc-fr1/pix-ui-review-pr846/environment

@theotime2005 theotime2005 marked this pull request as ready for review March 19, 2025 10:11
@xav-car
Copy link
Contributor

xav-car commented Mar 19, 2025

question

Nous n'aurions pas un problème avec l'affichage de x balise header dans le html ? Est ce qu'il ne vaudrait mieux pas que ce soit les différentes application qui inclus les PixBannerAlert à les disposer dans un header.

De plus la balise header va se retrouver écraser avec le role "alert" vu comment le composant est construit ?

PS : a noter qu'une evolutaion du PixAppLayout prévoir de regrouper les PixBannerAlert dans un élément centralisé. ( nous pourrions de ce fait lui faire porter l'informations du header

@theotime2005
Copy link
Contributor Author

question

Nous n'aurions pas un problème avec l'affichage de x balise header dans le html ? Est ce qu'il ne vaudrait mieux pas que ce soit les différentes application qui inclus les PixBannerAlert à les disposer dans un header.

De plus la balise header va se retrouver écraser avec le role "alert" vu comment le composant est construit ?

PS : a noter qu'une evolutaion du PixAppLayout prévoir de regrouper les PixBannerAlert dans un élément centralisé. ( nous pourrions de ce fait lui faire porter l'informations du header

Pour ce qui est de plusieur header, peut-être utiliser sinon d'autres balise comme article, mais quelque chose pour bien différencier les banières du reste. Je ne savais pas que toutes les banières allaient être regrouppées.

@xav-car
Copy link
Contributor

xav-car commented Mar 19, 2025

question
Nous n'aurions pas un problème avec l'affichage de x balise header dans le html ? Est ce qu'il ne vaudrait mieux pas que ce soit les différentes application qui inclus les PixBannerAlert à les disposer dans un header.
De plus la balise header va se retrouver écraser avec le role "alert" vu comment le composant est construit ?
PS : a noter qu'une evolutaion du PixAppLayout prévoir de regrouper les PixBannerAlert dans un élément centralisé. ( nous pourrions de ce fait lui faire porter l'informations du header

Pour ce qui est de plusieur header, peut-être utiliser sinon d'autres balise comme article, mais quelque chose pour bien différencier les banières du reste. Je ne savais pas que toutes les banières allaient être regrouppées.

l'article me semble plus approprié effectivement à la place de la div.

@lionelB
Copy link
Member

lionelB commented Mar 21, 2025

question: Qu'est ce que article apporte de plus que la div étant donné qu'un role="alert" est ajouté sur la div sachant qu'il écrase le role=article implicite ? d''ailleurs d'apres la doc mdn alert n'est pas autorisé comme role pour article (Rôles ARIA autorisés application, document, feed, main, none, presentation, region).

@theotime2005
Copy link
Contributor Author

question: Qu'est ce que article apporte de plus que la div étant donné qu'un role="alert" est ajouté sur la div sachant qu'il écrase le role=article implicite ?

Le but de article ou header et de baliser pour le lecteur d'écran, c'est tout. La div n'est pas vue comme un élément distinct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants