Story/character limit #56
Conversation
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-56.eastus2.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-56.eastus2.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-56.eastus2.azurestaticapps.net |
if user has other too long error and then unchecks other the error will go away all errors will appear to the user rather than just the first one
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-56.eastus2.azurestaticapps.net |
| question.type === "textarea" && | ||
| question.answer.length > TEXTAREA_MAX_CHAR | ||
| ) { | ||
| (validated = false), (question.charError = true); |
There was a problem hiding this comment.
Is there a reason we need this to be charError instead of just error? Also, I noticed that there are a lot of repeated checks going on between this inputsValidated function and the inputError function in TextboxList (for example, we check question.type here and input.type in TextboxList.js.) I feel like it would make things simpler if instead of question.error being a boolean, we set question.error to be the actual error message key or undefined if there is none. Then, instead of doing all these checks again in the inputError function and separating out error and charError, we could just do a simple
if (inputQuestions && inputQuestions[index] && inputQuestions[index].error) {
const key = inputQuestions[index].error;
return (
<ErrorAlert
key={key}
errorText={t(`errorMessages.${key}`)}
id={id}
className="margin-bottom-2"
/>
);
}
There was a problem hiding this comment.
Also, probably not a big deal, but the validated variable seems a little unnecessary to me and seems kind of redundant to have to set it in each if/else if body. I think it might be cleaner if we just return
return !inputQuestions.some(question => question.error);
at the very end
…ist to avoid repetition
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-56.eastus2.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-56.eastus2.azurestaticapps.net |
| } else if (!otherFieldValidated()) { | ||
| setOtherTooLong(true); | ||
| } | ||
| if (screen.textInputs && !inputsValidated()) { |
There was a problem hiding this comment.
Is there a reason this is an if instead of an else if? If there is a missing checkbox and an improper input at the same time, the focus should go to the checkbox since it comes before, but right now it goes to the input instead.
There was a problem hiding this comment.
If this is an else if then it will only show the checkbox error even if there are two errors
There was a problem hiding this comment.
I see— should we add an extra check for the checkboxError then, so that we're not accidentally focusing on the input instead of the checkbox? So if (screen.textInputs && !inputsValidated() && !checkboxError)
…re was an error for both
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-56.eastus2.azurestaticapps.net |
No description provided.