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

Feature/api 3866 corriger les trucs releve par audit #167

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

jbfeldis
Copy link
Collaborator

@jbfeldis jbfeldis commented Dec 6, 2024

Copy link

linear bot commented Dec 6, 2024

@jbfeldis jbfeldis marked this pull request as ready for review December 9, 2024 17:07
Copy link
Contributor

@DorineLam DorineLam left a comment

Choose a reason for hiding this comment

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

Merci ! 1 petit doute sur deux titles

@@ -84,7 +84,7 @@
<div class="fr-mb-1w">
<strong><%= t(".secondary_info.link_title") %></strong>
</div>
<%= link_to @qr_code_url, @qr_code_url, class: "break-link" %>
<%= link_to @qr_code_url, @qr_code_url, class: "break-link", title: "URL vers le formulaire de #{@collectivity.display_name}, cette page" %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem ici

@@ -88,20 +88,23 @@
<li>ne correspond pas aux exigences de sécurité ou porte préjudice à son image.</li>
</ul>
<p><strong>La </strong>DINUM<strong> s’engage à ce que le </strong>Formulaire QF<strong> et le </strong>Service<strong> soient accessibles à 99,5 % et à informer les </strong>Administrations<strong> de toute difficulté de nature à en compromettre le bon fonctionnement.
L’engagement de disponibilité porte sur le </strong>Formulaire QF<strong> et le </strong>Service**. Cet engagement ne s&#39;applique pas :</p>
L’engagement de disponibilité porte sur le </strong>Formulaire QF<strong> et le </strong>Service**.
</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Intéressant je ne savais pas qu'on pouvait mettre un <p> autour d'une liste <ul> !

Copy link
Contributor

Choose a reason for hiding this comment

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

Tout est possible, tout est imaginable, c'est le jeu de la vie ! :D

@@ -60,7 +60,7 @@

<div class="fr-grid-row">
<div class="fr-col-md-4 fr-col-12 fr-pr-md-2w">
<%= image_tag "data:image/png;base64,#{@qr_code}", alt: "QR code", class: "fr-responsive-img" %>
<%= image_tag "data:image/png;base64,#{@qr_code}", alt: "QR code vers le formulaire de #{@collectivity.display_name}, cette page", class: "fr-responsive-img" %>
Copy link
Contributor

Choose a reason for hiding this comment

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

@jbfeldis pour quelle raison c'est écrit ", cette page" à la fin du title ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

j'ai tenté un truc (mais j'ai posé la question à la personne qui a fait l'audit) 😄

je suis parti du principe que ces liens étaient étranges (pourquoi y'a tout ces liens qui ramènent sur la même page), donc je me suis dit qu'on pouvait ptet calquer la bonne pratique du "texte - nouvelle fenêtre" pour expliciter que ces liens et le QR code restaient sur la même page

Mais ptet qu'il va trouver ça naze 😅

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 c'est bizarre effectivement, on a l'impression qu'il manque des mots ^^

Copy link
Contributor

Choose a reason for hiding this comment

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

C'est en référence à quel feedback exactement que tu as tenté ça ?

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.

3 participants