Repository navigation
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
…anges Co-authored-by: TaduJR <TaduJR@users.noreply.github.com>
f709f14 to
ed00b6f
Compare
This comment has been minimized.
This comment has been minimized.
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/de.ts b/src/languages/de.ts
index 22213bec..89f21780 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -237,7 +237,7 @@ const translations: TranslationDeepObject<typeof en> = {
phoneNumber: `Bitte gib eine vollständige Telefonnummer ein
(z. B. ${CONST.FORMATTED_EXAMPLE_PHONE_NUMBER})`,
fieldRequired: 'Dieses Feld ist erforderlich',
- fixErrorsBeforeContinuing: 'Bitte behebe die Fehler im Formular, bevor du fortfährst.',
+ fixErrorsBeforeContinuing: 'Bitte beheben Sie die Fehler im Formular, bevor Sie fortfahren.',
requestModified: 'Diese Anfrage wird gerade von einem anderen Mitglied bearbeitet',
characterLimitExceedCounter: (length: number, limit: number) => `Zeichenlimit überschritten (${length}/${limit})`,
dateInvalid: 'Bitte wähle ein gültiges Datum aus',
@@ -1756,7 +1756,7 @@ const translations: TranslationDeepObject<typeof en> = {
atLeastTwoDifferentWaypoints: 'Bitte gib mindestens zwei verschiedene Adressen ein',
splitExpenseMultipleParticipantsErrorMessage: 'Eine Ausgabe kann nicht zwischen einem Workspace und anderen Mitgliedern aufgeteilt werden. Bitte aktualisiere deine Auswahl.',
invalidMerchant: 'Bitte gib einen gültigen Händler ein',
- enterRequiredFields: 'Bitte fülle die Pflichtfelder aus.',
+ enterRequiredFields: 'Bitte füllen Sie die erforderlichen Felder aus.',
atLeastOneAttendee: 'Mindestens eine teilnehmende Person muss ausgewählt werden',
invalidQuantity: 'Bitte gib eine gültige Menge ein',
quantityGreaterThanZero: 'Die Menge muss größer als null sein',
diff --git a/src/languages/el.ts b/src/languages/el.ts
index 4de669a7..d46f708d 100644
--- a/src/languages/el.ts
+++ b/src/languages/el.ts
@@ -241,7 +241,7 @@ const translations: TranslationDeepObject<typeof en> = {
phoneNumber: `Παρακαλούμε εισαγάγετε έναν πλήρη αριθμό τηλεφώνου
(π.χ. ${CONST.FORMATTED_EXAMPLE_PHONE_NUMBER})`,
fieldRequired: 'Αυτό το πεδίο είναι υποχρεωτικό',
- fixErrorsBeforeContinuing: 'Παρακαλώ διορθώστε τα σφάλματα στη φόρμα πριν συνεχίσετε.',
+ fixErrorsBeforeContinuing: 'Παρακαλούμε διορθώστε τα σφάλματα στη φόρμα πριν συνεχίσετε.',
requestModified: 'Αυτό το αίτημα τροποποιείται από άλλο μέλος',
characterLimitExceedCounter: (length: number, limit: number) => `Υπέρβαση ορίου χαρακτήρων (${length}/${limit})`,
dateInvalid: 'Παρακαλώ επιλέξτε έγκυρη ημερομηνία',
@@ -1824,7 +1824,7 @@ const translations: TranslationDeepObject<typeof en> = {
atLeastTwoDifferentWaypoints: 'Παρακαλούμε εισαγάγετε τουλάχιστον δύο διαφορετικές διευθύνσεις',
splitExpenseMultipleParticipantsErrorMessage: 'Μια δαπάνη δεν μπορεί να κατανεμηθεί μεταξύ ενός χώρου εργασίας και άλλων μελών. Παρακαλούμε ενημερώστε την επιλογή σας.',
invalidMerchant: 'Παρακαλούμε εισαγάγετε έγκυρο έμπορο',
- enterRequiredFields: 'Παρακαλούμε συμπληρώστε τα υποχρεωτικά πεδία.',
+ enterRequiredFields: 'Παρακαλούμε εισαγάγετε τα απαιτούμενα πεδία.',
atLeastOneAttendee: 'Πρέπει να επιλεγεί τουλάχιστον ένας συμμετέχων',
invalidQuantity: 'Παρακαλούμε εισαγάγετε μια έγκυρη ποσότητα',
quantityGreaterThanZero: 'Η ποσότητα πρέπει να είναι μεγαλύτερη από το μηδέν',
diff --git a/src/languages/es.ts b/src/languages/es.ts
index 5247d6c6..ebea2b85 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -228,7 +228,7 @@ const translations: TranslationDeepObject<typeof en> = {
acceptTerms: 'Debes aceptar los Términos de Servicio para continuar',
phoneNumber: `Por favor, introduce un número de teléfono completo\n(ej. ${CONST.FORMATTED_EXAMPLE_PHONE_NUMBER})`,
fieldRequired: 'Este campo es obligatorio',
- fixErrorsBeforeContinuing: 'Por favor, corrige los errores en el formulario antes de continuar.',
+ fixErrorsBeforeContinuing: 'Por favor, corrige los errores del formulario antes de continuar.',
requestModified: 'Esta solicitud está siendo modificada por otro miembro',
characterLimitExceedCounter: (length, limit) => `Se superó el límite de caracteres (${length}/${limit})`,
dateInvalid: 'Por favor, selecciona una fecha válida',
@@ -1762,7 +1762,7 @@ const translations: TranslationDeepObject<typeof en> = {
atLeastTwoDifferentWaypoints: 'Por favor, introduce al menos dos direcciones diferentes',
splitExpenseMultipleParticipantsErrorMessage: 'Solo puedes dividir un gasto entre un único espacio de trabajo o con miembros individuales. Por favor, actualiza tu selección.',
invalidMerchant: 'Por favor, introduce un comerciante válido',
- enterRequiredFields: 'Por favor, completa los campos obligatorios.',
+ enterRequiredFields: 'Por favor, introduce los campos obligatorios.',
atLeastOneAttendee: 'Debe seleccionarse al menos un asistente',
invalidQuantity: 'Por favor, introduce una cantidad válida',
quantityGreaterThanZero: 'La cantidad debe ser mayor que cero',
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index a3832449..0baeef38 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -1764,7 +1764,7 @@ const translations: TranslationDeepObject<typeof en> = {
atLeastTwoDifferentWaypoints: 'Veuillez saisir au moins deux adresses différentes',
splitExpenseMultipleParticipantsErrorMessage: "Une dépense ne peut pas être répartie entre un espace de travail et d'autres membres. Veuillez modifier votre sélection.",
invalidMerchant: 'Veuillez saisir un commerçant valide',
- enterRequiredFields: 'Veuillez remplir les champs obligatoires.',
+ enterRequiredFields: 'Veuillez renseigner les champs obligatoires.',
atLeastOneAttendee: 'Au moins un participant doit être sélectionné',
invalidQuantity: 'Veuillez saisir une quantité valide',
quantityGreaterThanZero: 'La quantité doit être supérieure à zéro',
diff --git a/src/languages/it.ts b/src/languages/it.ts
index 521bb9ab..97c933fa 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -237,7 +237,7 @@ const translations: TranslationDeepObject<typeof en> = {
phoneNumber: `Inserisci un numero di telefono completo
(ad es. ${CONST.FORMATTED_EXAMPLE_PHONE_NUMBER})`,
fieldRequired: 'Questo campo è obbligatorio',
- fixErrorsBeforeContinuing: 'Per favore, correggi gli errori nel modulo prima di continuare.',
+ fixErrorsBeforeContinuing: 'Correggi gli errori nel modulo prima di continuare.',
requestModified: 'Questa richiesta viene modificata da un altro membro',
characterLimitExceedCounter: (length: number, limit: number) => `Limite di caratteri superato (${length}/${limit})`,
dateInvalid: 'Seleziona una data valida',
@@ -1754,7 +1754,7 @@ const translations: TranslationDeepObject<typeof en> = {
atLeastTwoDifferentWaypoints: 'Inserisci almeno due indirizzi diversi',
splitExpenseMultipleParticipantsErrorMessage: 'Una spesa non può essere suddivisa tra uno spazio di lavoro e altri membri. Aggiorna la tua selezione.',
invalidMerchant: 'Inserisci un esercente valido',
- enterRequiredFields: 'Compila i campi obbligatori.',
+ enterRequiredFields: 'Inserisci i campi obbligatori.',
atLeastOneAttendee: 'Devi selezionare almeno un partecipante',
invalidQuantity: 'Inserisci una quantità valida',
quantityGreaterThanZero: 'La quantità deve essere maggiore di zero',
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index 140b1eb7..37706b88 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -237,7 +237,7 @@ const translations: TranslationDeepObject<typeof en> = {
phoneNumber: `Voer een volledig telefoonnummer in
(bijv. ${CONST.FORMATTED_EXAMPLE_PHONE_NUMBER})`,
fieldRequired: 'Dit veld is verplicht',
- fixErrorsBeforeContinuing: 'Corrigeer de fouten in het formulier voordat je verdergaat.',
+ fixErrorsBeforeContinuing: 'Los de fouten in het formulier op voordat je verdergaat.',
requestModified: 'Dit verzoek wordt bewerkt door een ander lid',
characterLimitExceedCounter: (length: number, limit: number) => `Limiet voor aantal tekens overschreden (${length}/${limit})`,
dateInvalid: 'Selecteer een geldige datum',
@@ -1750,7 +1750,7 @@ const translations: TranslationDeepObject<typeof en> = {
atLeastTwoDifferentWaypoints: 'Voer minimaal twee verschillende adressen in',
splitExpenseMultipleParticipantsErrorMessage: 'Een uitgave kan niet worden opgesplitst tussen een werkruimte en andere leden. Werk je selectie alsjeblieft bij.',
invalidMerchant: 'Voer een geldige leverancier in',
- enterRequiredFields: 'Vul de verplichte velden in.',
+ enterRequiredFields: 'Vul alsjeblieft de verplichte velden in.',
atLeastOneAttendee: 'Er moet ten minste één deelnemer worden geselecteerd',
invalidQuantity: 'Voer een geldige hoeveelheid in',
quantityGreaterThanZero: 'Hoeveelheid moet groter zijn dan nul',
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index a89e9c77..63787be4 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -243,7 +243,7 @@ const translations: TranslationDeepObject<typeof en> = {
phoneNumber: `Proszę podać pełny numer telefonu
(np. ${CONST.FORMATTED_EXAMPLE_PHONE_NUMBER})`,
fieldRequired: 'To pole jest wymagane',
- fixErrorsBeforeContinuing: 'Napraw błędy w formularzu przed kontynuowaniem.',
+ fixErrorsBeforeContinuing: 'Popraw błędy w formularzu przed kontynuowaniem.',
requestModified: 'Inny członek właśnie modyfikuje to żądanie',
characterLimitExceedCounter: (length: number, limit: number) => `Przekroczono limit znaków (${length}/${limit})`,
dateInvalid: 'Wybierz prawidłową datę',
@@ -1790,7 +1790,7 @@ const translations: TranslationDeepObject<typeof en> = {
atLeastTwoDifferentWaypoints: 'Wprowadź co najmniej dwa różne adresy',
splitExpenseMultipleParticipantsErrorMessage: 'Wydatek nie może być podzielony między przestrzeń roboczą a innych członków. Zaktualizuj swój wybór.',
invalidMerchant: 'Wprowadź prawidłowego sprzedawcę',
- enterRequiredFields: 'Wypełnij wymagane pola.',
+ enterRequiredFields: 'Wpisz wymagane pola.',
atLeastOneAttendee: 'Co najmniej jeden uczestnik musi zostać wybrany',
invalidQuantity: 'Wprowadź prawidłową ilość',
quantityGreaterThanZero: 'Ilość musi być większa niż zero',
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index c420680d..efe1f0a1 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -237,7 +237,7 @@ const translations: TranslationDeepObject<typeof en> = {
phoneNumber: `Insira um número de telefone completo
(p.ex., ${CONST.FORMATTED_EXAMPLE_PHONE_NUMBER})`,
fieldRequired: 'Este campo é obrigatório',
- fixErrorsBeforeContinuing: 'Por favor, corrija os erros no formulário antes de continuar.',
+ fixErrorsBeforeContinuing: 'Corrija os erros no formulário antes de continuar.',
requestModified: 'Esta solicitação está sendo modificada por outra pessoa',
characterLimitExceedCounter: (length: number, limit: number) => `Limite de caracteres excedido (${length}/${limit})`,
dateInvalid: 'Selecione uma data válida',
@@ -1751,7 +1751,7 @@ const translations: TranslationDeepObject<typeof en> = {
atLeastTwoDifferentWaypoints: 'Insira pelo menos dois endereços diferentes',
splitExpenseMultipleParticipantsErrorMessage: 'Uma despesa não pode ser dividida entre um espaço de trabalho e outros membros. Atualize sua seleção.',
invalidMerchant: 'Insira um comerciante válido',
- enterRequiredFields: 'Preencha os campos obrigatórios.',
+ enterRequiredFields: 'Digite os campos obrigatórios.',
atLeastOneAttendee: 'Pelo menos um participante deve ser selecionado',
invalidQuantity: 'Insira uma quantidade válida',
quantityGreaterThanZero: 'A quantidade deve ser maior que zero',
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index d969991e..52b7e30f 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -1687,7 +1687,7 @@ const translations: TranslationDeepObject<typeof en> = {
atLeastTwoDifferentWaypoints: '请输入至少两个不同的地址',
splitExpenseMultipleParticipantsErrorMessage: '无法将一笔报销同时分摊到一个工作区和其他成员。请更新你的选择。',
invalidMerchant: '请输入有效的商家',
- enterRequiredFields: '请填写必填字段。',
+ enterRequiredFields: '请输入必填字段。',
atLeastOneAttendee: '必须选择至少一位参加者',
invalidQuantity: '请输入有效的数量',
quantityGreaterThanZero: '数量必须大于零',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
This comment has been minimized.
This comment has been minimized.
|
Hi! We are aware Melvin tester runs are failing. Please if you could update this branch to latest main and re-run it - this should resolve the issue. If not, reach out to me directly. Thanks for your patience! |
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: TaduJR <TaduJR@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: TaduJR <TaduJR@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: TaduJR <TaduJR@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
|
Marked the PR as ready for review. |
|
@TaduJR @MelvinBot correct me if I wrong, but this became a big change because we are trying to not clear the error when we have more errors to fix?
|
|
Yes—your sequence is correct for the amount/date/merchant errors, but step 4 doesn’t inherently require this whole refactor. The central design choice was to derive those errors together, so submission blocking, inline errors, and the footer agree. Fixing one field removes only its error; the footer remains until none are left (implementation). The PR also includes additional behavior fixes:
So partial correction motivated the shared-error-map approach, but the additional fixes explain the broader scope. A narrower footer-only change was possible; this implementation chose to unify those validation paths rather than only expose the existing error string. |
|
@MelvinBot do we have this pattern somewhere? Or do we usually hide the footer error after the first fix and wait until next submission? |
|
Yes—we already have this pattern, but custom forms aren’t consistent. Based on the source:
So persistent feedback matches the shared-form default, while hide-until-resubmit also exists in custom flows. This supports the behavior chosen here, but doesn’t establish that this particular refactor—or its full scope—is necessary. |
trjExpensify
left a comment
There was a problem hiding this comment.
To recap the problem we were solving here:
- The user had more fields than the available scroll view.
- They scroll down and click "Create expense"
- The field above the scroll view gets an error
- The user doesn't see it and thinks the "Create expense" button is unresponsive.
@Expensify/design this PR seems to add back the error above the footer button at all times - but that was removed at some point I think, maybe duplicity? Are we okay with that, or do we only want the footer error to show there when a field in error is above the scroll view?
|
We probably removed it in the footer button because we usually don't show the footer error if there is only one error on screen. I think I would ultimately prefer it to scroll to the error rather than add another error because then there's technically two errors on screen which feels unnecessary. ScrollTo behaviors can be a bit iffy though, so if we're worried about that, then I think the footer button makes sense.
I think I'd prefer scrollTo over this since then we're already doing viewport detection stuff. |
|
Yeah, this was reported and observed on desktop as well. |
|
@MelvinBot please sync this PR with my branch again, and change nothing else.
Also mark this comment as resolved and hide it #106450 (review) |
Co-authored-by: TaduJR <TaduJR@users.noreply.github.com>
Co-authored-by: TaduJR <TaduJR@users.noreply.github.com>
|
Synced and pushed c250bf3, without force-pushing.
454 targeted tests and typecheck passed. Full-suite and final lint rerun timed out; Web verification was blocked by a blank page. view run · no recording available |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@MelvinBot please sync this PR with my branch again, and change nothing else.
|
Co-authored-by: TaduJR <TaduJR@users.noreply.github.com>
|
Synced and pushed 31e2e70.
39,519 tests passed, along with typecheck, lint, React Compiler, and Storybook. Web verification was unavailable because no authorized live session was available. |
|
Please replace the description with the below one DescriptionExplanation of ChangeWhen Create was blocked by a missing or invalid amount, merchant or date, the error only showed under that field. If the user had scrolled past it, pressing Create looked like it did nothing. Now a blocked Create scrolls the list to the first field that needs fixing (amount, then merchant, then date) so its error is in view. If the field is already on screen, the list stays put. If the field is taller than the visible area, the list scrolls to its bottom, where the error is. The field isn't focused, so the keyboard and the date picker don't pop open. Design preferred this over adding a second error above the button: #106450 (comment) To make this reliable, the amount, merchant and date errors are now worked out from the expense's current values instead of being saved and cleared in several places. That also fixes a few cases where errors disappeared or read wrong:
When scanning several receipts:
Fixed Issues$ #106237 Tests**Prerequisite: **
Test 1: Missing amount on a scanned receipt
Test 2: Scanned receipt with nothing typed in
Test 3: Missing fields on a manual expense
Test 4: Invalid merchant, and leaving the page
Test 5: Expense to a person
Test 6: Several receipts
Test 7: Editing a split whose receipt could not be read
Test 8: Normal distance expense
Offline testsSame as tests QA StepsSame as tests
PR Author Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / Safari |
Explanation of Change
When the amount, date or merchant blocked Create, the error only showed under that field. If the field was scrolled out of view, pressing Create looked like it did nothing.
Now a message shows above the button after a blocked press: "Please enter the required fields." when something is missing, or "Please fix the errors in the form before continuing." when nothing is missing but a value is invalid. The errors under the fields stay, and the message goes away once everything is fixed.
To keep that message correct, these three fields no longer share a single error. Their errors are worked out from the current values, and that one result blocks Create, shows the field errors and picks the message.
This changes a few things for users:
main, and the new message would have disappeared with it.(none)when editing a split to a person is now flagged as invalid, like on other expenses.The "fix the errors" line is now one translated sentence, and the two distance pages use it too.
Fixed Issues
$ #106237
PROPOSAL: #106237 (comment)
Tests
**Prerequisite: **
Test 1: Missing amount on a scanned receipt
Test 2: Scanned receipt with nothing typed in
Test 3: Missing fields on a manual expense
Test 4: Invalid merchant, and leaving the page
Test 5: Expense to a person
Test 6: Several receipts
Test 7: Editing a split whose receipt could not be read
Test 8: Normal distance expense
Offline tests
Same as tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari