-
Notifications
You must be signed in to change notification settings - Fork 11
Améliorer la gestion des fichiers dans la sidebar d’observation #1334
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
base: main
Are you sure you want to change the base?
Améliorer la gestion des fichiers dans la sidebar d’observation #1334
The head ref may contain hidden characters: "1305-am\u00E9liorer-la-gestion-des-fichiers-dans-la-sidebar-dobservation"
Conversation
confiture-web-app/src/components/tiptap/image/ImageImportPlugin.ts
Outdated
Show resolved
Hide resolved
848e310 to
a0860e8
Compare
64b7d7e to
1d6e6dc
Compare
1d6e6dc to
cf5997b
Compare
cf5997b to
9574f3b
Compare
| UPLOAD_ERROR_TIMEOUT = "Importation interrompue, délai d’attente dépassé. Vérifiez votre connexion et réessayez.", | ||
| UPLOAD_ERROR_UNKNOWN = "Importation échouée, erreur inconnue. Réessayez.", | ||
| UPLOAD_SUCCESS = "Fichier [FILE] ajouté.", | ||
| UPLOAD_SUCCESS_IMAGE = "Image [FILE] ajoutée.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
je me demande si c’est pas overkill de tout le temps inclure le nom du fichier dans le message 🤔
Genre si on upload un fichier avec un nom à rallonge Screenshot 2025-12-21 at 19.33.51 ou même 15af6606-81e9-43c4-a804-6e16138b6d6d.jpg (ce qui est pas si déconnant comme nom de fichier sur le web), est-ce que ça annonce "Image un-cinq-a-f-six-six-zérox-... [10 secondes de nom de fichier] ajoutée" ?
En plus, le seul mot qui différencie un ajout d’une suppression ("fichier xx ajouté" / "fichier xx supprimé") est à la fin du message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pour info :
- le succès d’upload (
UPLOAD_SUCCESS) n’est restitué que pour les aides techniques, car on suppose que l’apparition du nouveau fichier suffit à faire comprendre que l’upload a réussi. - le succès de delete (
DELETE_SUCCESS) est lui restitué visuellement avec un toast (et aux aides techniques bien sûr), car le fichier disparait donc on pourrait oublier le fichier qui vient d’être supprimé.
C’est sûr que ça peut faire long. Il me semble avoir discuté d’un raccourcissement du genre "début…fin.jpg" (?) Mais je ne sais pas trop…
Qu’en pense @AdrienMuzyczka ?
Pour "fichier xx ajouté" / "fichier xx supprimé") :
- « un seul mot qui différencie » : perso ça ne me dérange pas :)
- « à la fin du message » : j’entends l’argument. Effectivement ça serait peut-être bien d’inverser la tournure
Qu’en pense @AdrienMuzyczka ?
| ) { | ||
| inlineConfirmPendingRange.value = range; | ||
| await nextTick(); | ||
| await sleep(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on peut vraiment pas se passer de tout les sleep ? 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Celui-ci non : il permet d’annoncer le bouton "Supprimer" mis en focus.
Sans ça sur Safari avec VoiceOver on ne l’entends pas 🤷.
Je rajoute un commentaire dans le code 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
En revanche j’ai rework les anims en virant les autres sleep 👌
| }); | ||
|
|
||
| const emit = defineEmits<{ | ||
| (e: "file-deleted", payload: { resolve: (value: void) => void; flFile: FileListFile }): Promise<void>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pourquoi value: void plutôt que rien du tout ?
| (e: "file-deleted", payload: { resolve: (value: void) => void; flFile: FileListFile }): Promise<void>; | |
| (e: "file-deleted", payload: { resolve: () => void; flFile: FileListFile }): Promise<void>; |
Pareil pour toutes définitions de resolve
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eslint n’aime pas ça 🤷 :
Argument of type '(resolve: () => void) => void' is not assignable to parameter of type '(resolve: (value: unknown) => void, reject: (reason?: any) => void) => void'.
Types of parameters 'resolve' and 'resolve' are incompatible.
Target signature provides too few arguments. Expected 1 or more, but got 0.Le typage du constructeur dans l’interface de Promise (node_modules/typescript/lib/lib.es2015.promise.d.ts) :
new <T>(executor: (resolve: (value: T | PromiseLike<T>) => void, reject: (reason?: any) => void) => void): Promise<T>;|
|
||
| const notify = useNotifications(); | ||
|
|
||
| const maxFileSizeHumanReadable = computed(() => (props.maxFileSize / 1000000) + " Mo"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On peut mettre des _ dans les litterals de nombre, pratique pour rendre les grands nombres plus faciles à lire
| const maxFileSizeHumanReadable = computed(() => (props.maxFileSize / 1000000) + " Mo"); | |
| const maxFileSizeHumanReadable = computed(() => (props.maxFileSize / 1_000_000) + " Mo"); |
Il y’a une fonction formatBytes() qui fait déjà ça dans utils.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmmm… ok pour utiliser la fonction utilitaire, mais je vois qu’il s’agit d’une base 1024 et non 1000…
Je crois qu’il faudrait changer 1024 en 1000. Cf. https://fr.wikipedia.org/wiki/Pr%C3%A9fixe_binaire
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ça me va. tant que c’est pareil partout !
24a82bf to
b9e4a22
Compare
yaaax
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merci pour les retours.
J’ai fait pas mal de modifications.
Il y a 1 ou 2 questions en suspend dans les commentaires.
|
|
||
| const notify = useNotifications(); | ||
|
|
||
| const maxFileSizeHumanReadable = computed(() => (props.maxFileSize / 1000000) + " Mo"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmmm… ok pour utiliser la fonction utilitaire, mais je vois qu’il s’agit d’une base 1024 et non 1000…
Je crois qu’il faudrait changer 1024 en 1000. Cf. https://fr.wikipedia.org/wiki/Pr%C3%A9fixe_binaire
| UPLOAD_ERROR_TIMEOUT = "Importation interrompue, délai d’attente dépassé. Vérifiez votre connexion et réessayez.", | ||
| UPLOAD_ERROR_UNKNOWN = "Importation échouée, erreur inconnue. Réessayez.", | ||
| UPLOAD_SUCCESS = "Fichier [FILE] ajouté.", | ||
| UPLOAD_SUCCESS_IMAGE = "Image [FILE] ajoutée.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pour info :
- le succès d’upload (
UPLOAD_SUCCESS) n’est restitué que pour les aides techniques, car on suppose que l’apparition du nouveau fichier suffit à faire comprendre que l’upload a réussi. - le succès de delete (
DELETE_SUCCESS) est lui restitué visuellement avec un toast (et aux aides techniques bien sûr), car le fichier disparait donc on pourrait oublier le fichier qui vient d’être supprimé.
C’est sûr que ça peut faire long. Il me semble avoir discuté d’un raccourcissement du genre "début…fin.jpg" (?) Mais je ne sais pas trop…
Qu’en pense @AdrienMuzyczka ?
Pour "fichier xx ajouté" / "fichier xx supprimé") :
- « un seul mot qui différencie » : perso ça ne me dérange pas :)
- « à la fin du message » : j’entends l’argument. Effectivement ça serait peut-être bien d’inverser la tournure
Qu’en pense @AdrienMuzyczka ?
| }); | ||
|
|
||
| const emit = defineEmits<{ | ||
| (e: "file-deleted", payload: { resolve: (value: void) => void; flFile: FileListFile }): Promise<void>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eslint n’aime pas ça 🤷 :
Argument of type '(resolve: () => void) => void' is not assignable to parameter of type '(resolve: (value: unknown) => void, reject: (reason?: any) => void) => void'.
Types of parameters 'resolve' and 'resolve' are incompatible.
Target signature provides too few arguments. Expected 1 or more, but got 0.Le typage du constructeur dans l’interface de Promise (node_modules/typescript/lib/lib.es2015.promise.d.ts) :
new <T>(executor: (resolve: (value: T | PromiseLike<T>) => void, reject: (reason?: any) => void) => void): Promise<T>;| ) { | ||
| inlineConfirmPendingRange.value = range; | ||
| await nextTick(); | ||
| await sleep(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Celui-ci non : il permet d’annoncer le bouton "Supprimer" mis en focus.
Sans ça sur Safari avec VoiceOver on ne l’entends pas 🤷.
Je rajoute un commentaire dans le code 🙏
| ) { | ||
| inlineConfirmPendingRange.value = range; | ||
| await nextTick(); | ||
| await sleep(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
En revanche j’ai rework les anims en virant les autres sleep 👌
And use it in AuditGenerationCriterium.vue and NotesModal.vue
* file deletion: add an inline confirm stage within the NotesModal * convey file upload and file deletion to assistive technologies * remove the visual success message * better handle focus after deletion
* emit events instead of passing callback functions as props * properly handle file upload / delete notifications conveyed to assistive technologies * sleep or nextTick only when needed
* to screen readers only in Notes * with a toast in criteria Make sure it works on Safari with a 500ms delay
+ clarify "all files" label (visually and for assistive technologies)
(removes risky querySelector)
And use this utility function in FileList component
Full deprecated route path is: "/api/audits/{uniqueId}/results/examples"
It’s not a type
… to prevent animating FileList items from overlapping other NotesModal elements
Use getFileMessage everywhere
This function is not used anymore.
Animations: * do not remove <ul> element when file list is empty so that first list item appears/disappears correctly * remove unneeded delays (calls to sleep function) * add a <div class="file-item"> element inside <li> element to allow the <li> absolute position while transitionning Inline delete confirmation panel: * focus "Cancel" button first instead of "Confirm delete" button to prevent unwanted deletes * disable file item Delete button when confirm panel is open
8229c83 to
7f0ca1e
Compare
closes #1305
Avant de merger la pull request, s’assurer que :