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

NewUpload(ui): make user list table larger #2690

Merged
merged 5 commits into from
Apr 1, 2025

Conversation

Holist
Copy link
Collaborator

@Holist Holist commented Feb 27, 2025

close #2671

Dans cette PR :

  • La taille des tableaux est agrandie.
  • Les emails ne sont plus tronqués.
  • Le footer est positionné en absolute en bas de la page.
Enregistrement.de.l.ecran.2025-03-19.a.17.43.53.mp4

@Holist Holist changed the title NewUpload(ui): make user list table more large NewUpload(ui): make user list table larger Feb 27, 2025
@Holist Holist self-assigned this Feb 27, 2025
Copy link
Collaborator

@aminedhobb aminedhobb left a comment

Choose a reason for hiding this comment

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

Merci @Holist , j'ai quelques réflexions suite à la review:

  • je pense qu'on peut ne rien préciser comme taille de largeur dans la plupart des cas
  • le fait de garder le footer en bas de page quelque soit la taille de la liste n'est pas traité il me semble
  • je crois qu'il était demandé aussi de ne plus tronquer les emails

@Holist Holist marked this pull request as draft February 27, 2025 14:36
@@ -1,6 +1,6 @@
<tr>
<% [:title, :first_name, :last_name, :affiliation_number, :phone_number, :email, :post_code].each do |attribute| %>
<td class="<%= "text-truncate-150" if attribute.in?([:email, :affiliation_number]) %>">
<td class="<%= "text-truncate-150" if attribute.in?([:affiliation_number]) %>">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Je préfère garder la syntaxe initiale dans le cas ou on aurait d'autres attributs a truncate.

@Holist Holist marked this pull request as ready for review March 19, 2025 16:51
@Holist
Copy link
Collaborator Author

Holist commented Mar 19, 2025

Merci @Holist , j'ai quelques réflexions suite à la review:

  • je pense qu'on peut ne rien préciser comme taille de largeur dans la plupart des cas
  • le fait de garder le footer en bas de page quelque soit la taille de la liste n'est pas traité il me semble
  • je crois qu'il était demandé aussi de ne plus tronquer les emails

C'est fait :)

Copy link
Collaborator

@aminedhobb aminedhobb left a comment

Choose a reason for hiding this comment

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

Merci @Holist , j'ai laissé deux remarques sur le footer et sur la page du dépôt du fichier.

@samanthaauffret pour info le fait de ne plus couper les mails long fait qu'on a encore le risque que le tableau nécessite un scroll horizontal. C'est ce que j'ai en tous cas quand je fais un test en local :
image

Est-ce qu'on ne devrait pas garder la troncation du mail et possiblement l'afficher entièrement si on passe la souris par dessus ?

@@ -41,7 +41,7 @@
}

.sticky-footer {
position: sticky;
position: absolute;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Avec ce changement on doit scroller verticalement pour accéder au footer sur les grands tableaux.
Moi ça ne me dérange pas tant mais je pense que c'était pas la vision qu'avait Samantha

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Je comprend pas. J'ai chargé un fichier de plus de 100 usagers et je n'arrive pas à reproduire le comportement que tu décris. Le footer est toujours visible en bas de page sans avoir besoin de scroll.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Si vos tests sont concluants sur l'affichage complets des adresses emails, gardons le comme ça.
On les avaient tronqué pour réduire les risques sur petits écrans. Mais j'imagine qu'avec l'agrandissement du tableau en largeur, le risque que les statuts de dossier soit invisible est réduit, non ? @Holist , @aminedhobb

Copy link
Collaborator

Choose a reason for hiding this comment

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

Chez moi j'ai besoin de scroll un peu pour le voir entièrement. Ceci dit si c'est bon pour vous mergeons comme ça et on pourra réajuster après au pire.

Donnees.usagers.chargees.-.Conseil.departemental.du.Finistere.-.rdv-insertion.3.webm

@Holist
Copy link
Collaborator Author

Holist commented Mar 21, 2025

Merci @Holist , j'ai laissé deux remarques sur le footer et sur la page du dépôt du fichier.

@samanthaauffret pour info le fait de ne plus couper les mails long fait qu'on a encore le risque que le tableau nécessite un scroll horizontal. C'est ce que j'ai en tous cas quand je fais un test en local : image

Est-ce qu'on ne devrait pas garder la troncation du mail et possiblement l'afficher entièrement si on passe la souris par dessus ?

Ca me parait quand même exceptionnellement long comme email non ?

Copy link
Collaborator

@aminedhobb aminedhobb left a comment

Choose a reason for hiding this comment

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

Ca me parait quand même exceptionnellement long comme email non ?

Sur des longues listes je dirai pas forcément, mais là encore on peut essayer comme ça et réajuster après si besoin.

@@ -41,7 +41,7 @@
}

.sticky-footer {
position: sticky;
position: absolute;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Chez moi j'ai besoin de scroll un peu pour le voir entièrement. Ceci dit si c'est bon pour vous mergeons comme ça et on pourra réajuster après au pire.

Donnees.usagers.chargees.-.Conseil.departemental.du.Finistere.-.rdv-insertion.3.webm

@Holist Holist merged commit 01ad853 into staging Apr 1, 2025
6 checks passed
@Holist Holist deleted the improve_user_upload_ui_table_display branch April 1, 2025 08:08
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.

[Upload] - Agrandir la zone d'affichage des données
3 participants