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.

Sign up for GitHub

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

Jump to bottom

fix: Confirm Dialog allows badly formatter code to be submitted #3072

Merged
merged 5 commits into from
Jan 5, 2024
Merged

fix: Confirm Dialog allows badly formatter code to be submitted #3072

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3d30165
disable submit button if error detected
wmoussa-gc Jan 3, 2024
6cc56bd
using named tag for Trans component
wmoussa-gc Jan 3, 2024
c55c988
modifying the dialog content based on designer input
wmoussa-gc Jan 3, 2024
e4610a4
reset error onpaste
wmoussa-gc Jan 5, 2024
a0a83ea
Merge branch 'develop' into bug-fix-confirmation
wmoussa-gc Jan 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions components/form-builder/app/responses/Dialogs/ConfirmDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { useState } from "react";
import { useTranslation } from "next-i18next";
import { useTranslation, Trans } from "next-i18next";
import { useDialogRef, Dialog } from "@components/form-builder/app/shared";
import { LineItemEntries } from "./line-item-entries";
import { Button, Alert } from "@components/globals";
Expand Down Expand Up @@ -149,9 +149,12 @@ export const ConfirmDialog = ({
{t("downloadResponsesModals.confirmReceiptDialog.errors.invalidEntry.title")}
</Alert.Title>
<p>
{t(
"downloadResponsesModals.confirmReceiptDialog.errors.invalidEntry.description"
)}
<Trans
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to use the "t" function here

const { t } = useTranslation("name-of-file");

i.e.

t("downloadResponsesModals.confirmReceiptDialog.errors.minEntries.title")

Copy link
Contributor Author

@wmoussa-gc wmoussa-gc Jan 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I know, but I took it up a notch, and now you've got a recipe that lets you style "some words" in your translation string with some basic HTML elements (italic in this case). As far as I've read and tested, the t function doesn't support it. Trans component does it but albeit not super intuitively https://react.i18next.com/latest/trans-component

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please let me know if you think it is a bad idea

Copy link
Member

@timarney timarney Jan 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not opposed to it ... to date we've broken strings apart which isn't a nice experience.

For sure it's not intuitive and the code without reading the docs looks like a mistake i.e. the empty <i></i>

Maybe leave a comment above the i tags to help any confusion on that end ... just something helpful for the next dev to edit that code.

Can we try using named tags? Might be helpful for folks doing the translations (thoughts?)

Make sure you also adapt your translation resources to include the named tags () instead of the indexed tags (<0>)!

Copy link
Member

Choose a reason for hiding this comment

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

import { Trans } from "next-i18next";

Re: the import + using Trans

@bryan-robitaille any concerns thinking about the Next JS update + translations 👆

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't think of any conflicts at the moment. In the Next JS update the i18n is initialized separately on the server and the client and the next-i18next package is removed. We'll be using pure react-i18next and it 'should' work as long as the initialized t function is passed in.
I completely agree about using named tags as the indexes are not very straight forward and can quickly change when content is updated.

ns="form-builder-responses"
i18nKey="downloadResponsesModals.confirmReceiptDialog.errors.invalidEntry.description"
defaults="<italic></italic>" // indicate to translator: text with italic HTML element
components={{ italic: <i /> }}
></Trans>
</p>
</Alert.Danger>
)}
Expand Down Expand Up @@ -186,10 +189,13 @@ export const ConfirmDialog = ({
)}
</div>
<div className="py-4">
<h4>{t("downloadResponsesModals.confirmReceiptDialog.contentHeading")}</h4>
<p>{t("downloadResponsesModals.confirmReceiptDialog.contentBody")}</p>
<p className="mb-2 mt-10 font-bold" id={confirmInstructionId}>
{t("downloadResponsesModals.confirmReceiptDialog.copyCode")}
<p className="font-bold" id={confirmInstructionId}>
<Trans
ns="form-builder-responses"
i18nKey="downloadResponsesModals.confirmReceiptDialog.copyCode"
defaults="<italic></italic>" // indicate to translator: text with italic HTML element
components={{ italic: <i /> }}
></Trans>
</p>
<p className="mb-4">
{t("downloadResponsesModals.confirmReceiptDialog.copyCodeNote")}
Expand All @@ -206,11 +212,19 @@ export const ConfirmDialog = ({
setStatus={setStatus}
></LineItemEntries>

<p className="mb-2 mt-10 font-bold">
{t("downloadResponsesModals.confirmReceiptDialog.contentHeading")}
</p>
<p>{t("downloadResponsesModals.confirmReceiptDialog.contentBody")}</p>

<div className="mt-4 flex gap-4">
<Button theme="secondary" onClick={handleClose}>
{t("downloadResponsesModals.cancel")}
</Button>
<Button onClick={handleSubmit} disabled={status === DialogStates.SENDING}>
<Button
onClick={handleSubmit}
disabled={status === DialogStates.SENDING || status === DialogStates.FORMAT_ERROR}
>
{status === DialogStates.SENDING
? t("downloadResponsesModals.sending")
: t("downloadResponsesModals.confirmReceiptDialog.confirmReceipt")}
Expand Down
29 changes: 16 additions & 13 deletions components/form-builder/app/responses/Dialogs/line-item-entries/LineItemEntries.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,21 @@ export const LineItemEntries = ({
}
};

const onPaste = (e: React.ClipboardEvent<HTMLInputElement>) => {
setStatus(DialogStates.EDITING); // Reset any errors
const pastedText = e.clipboardData.getData("Text");
const pastedTextArray = pastedText.split(/\r?\n/);
const cleanedText = pastedTextArray.flatMap((text) => {
const cleanedText = text.trim().replace(",", "").replaceAll("\t", "").toLowerCase();
if (validateInput && !validateInput(cleanedText)) {
setStatus(DialogStates.FORMAT_ERROR);
}
return cleanedText;
});
setInputs([...new Set([...inputs, ...cleanedText])]);
e.preventDefault();
};

useEffect(() => {
scrollToBottom(containerRef?.current as HTMLElement);
}, [inputs]);
Expand Down Expand Up @@ -133,19 +148,7 @@ export const LineItemEntries = ({
onBlur={onBlur}
spellCheck="false"
autoComplete="off"
onPaste={(e) => {
const pastedText = e.clipboardData.getData("Text");
const pastedTextArray = pastedText.split(/\r?\n/);
const cleanedText = pastedTextArray.flatMap((text) => {
const cleanedText = text.trim().replace(",", "").replaceAll("\t", "").toLowerCase();
if (validateInput && !validateInput(cleanedText)) {
setStatus(DialogStates.FORMAT_ERROR);
}
return cleanedText;
});
setInputs([...new Set([...inputs, ...cleanedText])]);
e.preventDefault();
}}
onPaste={onPaste}
aria-labelledby={inputLabelId}
/>
</div>
Expand Down
10 changes: 5 additions & 5 deletions public/static/locales/en/form-builder-responses.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,10 @@
"confirmReceiptDialog": {
"title": "Sign off on the removal of responses from GC Forms",
"contentHeading": "GC Forms requires verification that responses were downloaded successfully",
"contentBody": "Why? Because things can go wrong when digital files move between computers. This step helps to verify that all files made it safely to your computer from GC Forms.",
"copyCode": "Copy-paste receipt codes from the 'Official receipt and record of responses' to confirm you have responses.",
"copyCodeNote": "Use the “Copy receipt codes” button at the top of the receipt file.",
"confirmReceipt": "Sign off on removal of responses",
"contentBody": "Why? Because things can go wrong when digital files move between computers. This step helps to verify that all files made it safely to your computer.",
"copyCode": "Copy-paste receipt codes from the <italic>Official Receipt and Record</italic> file",
"copyCodeNote": "Use button at the top of the receipt file you got with the downloaded responses",
"confirmReceipt": "Submit receipt codes",
"errors": {
"minEntries": {
"title": "At least one response must be specified",
Expand All @@ -193,7 +193,7 @@
},
"invalidEntry": {
"title": "The receipt code does not match the required format",
"description": "Check that the receipt code format is correct and try again."
"description": "Receipt codes for sign off are found in the <italic>Official receipt and record</italic> file that downloaded with your responses."
},
"unknown": {
"title": "Sorry, there’s a problem with the site",
Expand Down
10 changes: 5 additions & 5 deletions public/static/locales/fr/form-builder-responses.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,10 @@
"confirmReceiptDialog": {
"title": "Approuver la suppression des réponses de Formulaires GC",
"contentHeading": "Formulaires GC doit vérifier que les réponses ont bien été téléchargées.",
"contentBody": "Pourquoi? Parce que les choses peuvent mal tourner lorsque des fichiers numériques passent d’un ordinateur à un autre. Cette étape sert à vérifier que les fichiers ont été téléchargés en toute sécurité sur votre ordinateur de Formulaires GC.",
"copyCode": "Copiez-collez les codes de réception du « Reçu et registre officiel des réponses » pour confirmer que vous avez bien reçu les réponses.",
"copyCodeNote": "Utilisez le bouton « Copier les codes de réception » en haut du fichier de reçu HTML.",
"confirmReceipt": "Approuver la suppression de réponses",
"contentBody": "Pourquoi? Parce que les choses peuvent mal tourner lorsque des fichiers numériques passent d’un ordinateur à un autre. Cette étape sert à vérifier que les fichiers ont été téléchargés en toute sécurité sur votre ordinateur.",
"copyCode": "Copiez-collez les codes de réception du fichier <italic>Reçu et registre officiel des réponses</italic>",
"copyCodeNote": "Utilisez le bouton en haut du fichier de reçu que vous avez eu avec les réponses téléchargées.",
"confirmReceipt": "Soumettre les codes de réception",
"errors": {
"minEntries": {
"title": "Au moins une réponse doit être spécifiée",
Expand All @@ -193,7 +193,7 @@
},
"invalidEntry": {
"title": "Le code de réception ne correspond pas au format requis",
"description": "Vérifiez que le format du code de réception est correct et réessayez."
"description": "Les codes de réception pour l'approbation se trouvent dans le fichier <italic>Reçu et registre officiel</italic> qui a été téléchargé avec vos réponses."
},
"unknown": {
"title": "Désolé, il y a un problème avec le site",
Expand Down
Loading