Conversation
There was a problem hiding this comment.
Pull request overview
Fixes CMS-1952 by refining validation error display and submission-state handling.
Changes:
- Adds conditional submit-error UI visibility.
- Updates validation summary and bypass controls.
- Prevents draft saves from marking forms as submitted.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
duncan-oxd
left a comment
duncan-oxd
left a comment
There was a problem hiding this comment.
Looks good overall, but I had some questions
| <Offcanvas.Body> | ||
| <div id="validation-errors"> | ||
| {validation.errors.length > 0 && ( | ||
| {shouldShowSubmitErrorUI && ( |
There was a problem hiding this comment.
I know the task says not to show the ErrorSummary component if the only error is the internal notes, but I still think we should show it. Mike brought it up on the call the other day and I don't know if we ever reached a consensus though.
If the only error is ‘Internal notes’ do not show ‘Submit with errors’ functionality (Summary and submit anyways)
It might be worth double-checking with UX, because it would simplify the changes too!
There was a problem hiding this comment.
Updated the behaviour to keep ErrorSummary visible after submit/approve attempts even when internal notes is the only error.
There was a problem hiding this comment.
Nice! The recent changes simplified this branch a lot. I put in a note about renaming one of the useMemo variables, since "hasNonNotesErrors" could be false because of the shouldShowErrorSummary dependency, even if there are non-note errors, and that could get confusing.
Otherwise, LGTM!
| return validation.errors.length > 0; | ||
| }, [submitted, validation.errors]); | ||
|
|
||
| const hasNonNotesErrors = useMemo(() => { |
There was a problem hiding this comment.
This is being used more like a shouldShowSubmitAnyway variable so it would clarify things to rename it to something like that, and add a couple of comments to say what we're using it for 👍
Jira Ticket
CMS-1952
Description
shouldShowSubmitErrorUIflag that ensures the summary error UI is only shown for validation errors other than internal notes.shouldShowSubmitErrorUIinstead of checking for any validation errors, preventing unnecessary or redundant error displays.saveFormfunction so that the form is only marked as "submitted" (which triggers error UI) when the status is notREQUESTED, aligning error display with actual submission attempts.