-
Notifications
You must be signed in to change notification settings - Fork 92
feat: Ensure "Go to review" is always displayed for forms #10314
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: develop
Are you sure you want to change the base?
Conversation
| {continueButtonText} | ||
| </Button> | ||
|
|
||
| {showReviewButton && ( |
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.
Is the showReviewButton being used for something else? If not can we remove it then?
| description: 'Back button text', | ||
| id: 'v2.buttons.back' | ||
| }, | ||
| backToReview: { |
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.
| backToReview: { | |
| goToReview: { |
| )} | ||
| </Button> | ||
| ), | ||
| declaration.review && ( |
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.
Do we want the same change in v1 too?
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.
Thanks @Zangetsu101, what do you mean by v1? I thought this is v1
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.
Sorry let me rephrase that, do we need the change in v1? Or just having it in v2 is enough?
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.
Thanks @Zangetsu101 I'm not sure about this detail, @euanmillar can you weigh in please. My thinking was that I needed to do it for both v1 and v2 to keep things consistent but since Residence Registration is a v2 event I think it makes sense to to only make the change on v2 side
|
This PR has been marked with label stale Since it has been inactive for 20 days. It will automatically be closed in 10 days if no further activity occurs. |
f2e2b31 to
b24039a
Compare
| formPages={formPages} | ||
| pageId={currentPageId} | ||
| setFormData={(data) => setAnnotation(data)} | ||
| showReviewButton={false} |
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.
Do we want to show the review button during the initial correction form too? There are two forms during the correction flow, one that collects details regarding the correction and the other is the usual declaration form where we make the corrections. This is the first one so let's make sure we are consciously removing it
| })} | ||
| pageId={currentPageId} | ||
| setFormData={(data) => setAnnotation(data)} | ||
| showReviewButton={searchParams.from === 'review'} |
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.
similarly for this one, during print certificate action we do not even have a review page so wouldn't make sense to show the button
| const { draft, pageRoute, writeDeclaration } = this.props | ||
| const declaration = draft | ||
| declaration.review = true | ||
| writeDeclaration(declaration) |
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.
is this intended?
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.
Yes it is, otherwise the tests fail for some reason the in-memory storage was not working well in testing environment since we re-routing
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.
That's a bit weird as we aren't really making any changes on the v1 side 🤔
|
@Siyasanga is this still relevant? Can we close this |
@rikukissa yes it is still relevant since the Household members list still accepts up to 30 members so for big households we'll still need a way to jump to the review screen It was blocked by the unrelated failing |
- Not persisting was causing errors within the tests for packages/client/src/views/RegisterForm/DeclarationForm.test.tsx ` renders preview page test - Also add back the translations for v1 to still work #10132
We don't need to show the "Go to review" button on metadata form like certificate collector's form and correction request forms. #10132
2a4b652 to
5ad3386
Compare
|
@Siyasanga which release should this go to? |
|
This PR has been marked with label stale Since it has been inactive for 20 days. It will automatically be closed in 10 days if no further activity occurs. |
Addresses: #10132
Country-config PR: opencrvs/opencrvs-countryconfig#959
Description
Clearly describe what has been changed. Include relevant context or background.
Explain how the issue was fixed (if applicable) and the root cause.
Link this pull request to the GitHub issue (and optionally name the branch
ocrvs-<issue #>)Checklist
Note
Always display a "Go to review" button across event forms by default, replacing "Back to review" and updating i18n, navigation, components, and tests accordingly.
showReviewButtontotrueinv2-events/features/events/components/Pages.tsx; remove conditionalshowReviewButtonpassing fromDECLARE/REGISTER/VALIDATE/REQUEST_CORRECTIONpages; explicitly setshowReviewButton=falseforPRINT_CERTIFICATE."Back to review"→"Go to review"inFormWizard,VerificationWizard, and all usages; update button ids/keys.messages.goToReviewButtonini18n/messages/views/register.ts; adjust references throughout.ReviewSectionto persist (writeDeclaration) before navigating from review edits."Back to review"with"Go to review"across multiple Storybook tests.Written by Cursor Bugbot for commit 0507adf. This will update automatically on new commits. Configure here.