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.

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

feat: 1361 add cancel button at seedlot review page #1452

Merged
merged 24 commits into from
Aug 1, 2024

Conversation

craigyu
Copy link
Collaborator

@craigyu craigyu commented Jul 25, 2024

Description

Closes #1361
!mportant:
Need to nuke the a-class-seedlot-save table, or these changes will cause trouble with existing drafts

Not described in the issue:
Refactored the behaviour of A-Class seedlot Context and ApplicantAgencyFields, the idea is let one component to handle the data fetching, the component displays the client acronym but stores client number. Renamed things from agency to client so it's consistent with forest client api.

  • Renamed to ClientAndCodeInput
  • default checkbox prop is now optional

Changelog

New

  • react-beforeunload allows confirmation before url change
  • Back to review / Back button on Seedlot Review page
  • Modal that confirms user's action
  • shameless plugin with my own photo 🥷

Changed

  • ApplicantAgencyFields -> ClientAndCodeInput
  • New pictogram for Review Seedlot card (NOT Seedlot Review)
  • Cypress tests

Removed

  • None

How was this tested?

  • 🧠 Not needed
  • 👀 Eyeball
  • 🤖 Added tests

What gif/image best describes this PR or how it makes you feel?


Thanks for the PR!

Deployments, as required, will be available below:

Please create PRs in draft mode. Mark as ready to enable:

After merge, new images are deployed in:

@craigyu craigyu added the front-end front-end related task label Jul 25, 2024
@craigyu craigyu requested a review from ngunner15 July 25, 2024 22:54
@craigyu craigyu self-assigned this Jul 25, 2024
@craigyu craigyu linked an issue Jul 25, 2024 that may be closed by this pull request
@craigyu craigyu changed the title Feat/1361 add cancel button at seedlot review page feat: 1361 add cancel button at seedlot review page Jul 29, 2024
@craigyu
Copy link
Collaborator Author

craigyu commented Jul 29, 2024

fixing Cypress test atm

craigyu added 4 commits July 30, 2024 10:18
…1-add-cancel-button-at-seedlot-review-page

# Conflicts:
#	frontend/src/components/LotApplicantAndInfoForm/constants.ts
#	frontend/src/components/LotApplicantAndInfoForm/index.tsx
#	frontend/src/components/SeedlotRegistrationSteps/CollectionStep/index.tsx
#	frontend/src/components/SeedlotRegistrationSteps/ExtractionAndStorageStep/index.tsx
#	frontend/src/components/SeedlotRegistrationSteps/InterimStep/index.tsx
#	frontend/src/components/SeedlotReviewSteps/ApplicantAndSeedlot/Read/index.tsx
#	frontend/src/components/SeedlotReviewSteps/Collection/Read/index.tsx
#	frontend/src/components/SeedlotReviewSteps/ExtractionStorage/Read/index.tsx
#	frontend/src/components/SeedlotReviewSteps/Interim/Read/index.tsx
#	frontend/src/components/SeedlotReviewSteps/Ownership/Read/index.tsx
Copy link
Collaborator

@mgaseta mgaseta left a comment

Choose a reason for hiding this comment

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

LGTM!
Added only one small mark-up.

Copy link
Contributor

@RMCampos RMCampos left a comment

Choose a reason for hiding this comment

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

Great job! Some typos, nothing more.

🥇 🚀

import { ComboBoxPropsType } from './definitions';

export const agencyFieldsText = (isReview: boolean | undefined): AgencyTextPropsType => ({
export const clientAndCodeInputText = (isReview: boolean | undefined): ClientAndCodeInputTextType => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

lint issue:

~/Projects/nr-spar/frontend/src/components/LotApplicantAndInfoForm/constants.ts
  5:1  error  This line has a length of 103. Maximum allowed is 100  max-len

/**
* Format the displayed location code
*/
const formatDisplayedLocCode = (formatedCode: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: formated instead of formatted

/**
* Format the displayed acronym
*/
const formatDisplayedAcronym = (formatedAcronym: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: formated instead of formatted

};

const handleLocationCodeBlur = (value: string) => {
const formatedCode = value ? formatLocationCode(value) : '';
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: formated instead of formatted

@@ -54,7 +54,7 @@ const AuditInfo = () => {
id="last-updated-at"
label="Last uptated at:"
value={
seedlotData
seedlotData?.auditInformation.updateTimestamp
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: uptated instead of updated (twice, lines 47 and 55. Not changed by you, tho)

max-width: none;
width: -webkit-fill-available;
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

question (non-blocking): I'm getting a warning at line 46, asking to define: Also define the standard property 'appearance' for compatibilityscss(vendorPrefix)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm, what's the suggested fix? does it have one?

@@ -86,14 +87,23 @@ describe('A Class Seedlot Registration form, Orchard', () => {
.contains('219 - VERNON - S - PRD')
.click();

// Intercept the call before step 5 monuts
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: mounts

@@ -225,14 +235,22 @@ describe('A Class Seedlot Registration form, Orchard', () => {
.contains('219 - VERNON - S - PRD')
.click();

// Intercept the call before step 5 monuts
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: mounts

@@ -281,14 +299,22 @@ describe('A Class Seedlot Registration form, Orchard', () => {
.contains('222 - VERNON - S - PRD')
.click();

// Intercept the call before step 5 monuts
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: mounts

@@ -354,14 +380,22 @@ describe('A Class Seedlot Registration form, Orchard', () => {
// Save changes
cy.saveSeedlotRegFormProgress();

// Intercept the call before step 5 monuts
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: mounts

@craigyu craigyu requested review from ngunner15 and RMCampos August 1, 2024 04:53
@craigyu
Copy link
Collaborator Author

craigyu commented Aug 1, 2024

@RMCampos @ngunner15

Alright guys, it looks like I can't spell for 💩
here are the updates:

  • Typo and lint: 50ba266
  • Back modal update 2851cf9
  • Confirm modal when saving and update the status f6533f4
  • Also fixed a problem where ownership's default method of payment is sometimes not filled

Copy link
Collaborator

@ngunner15 ngunner15 left a comment

Choose a reason for hiding this comment

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

LGTM!

@craigyu craigyu closed this Aug 1, 2024
@craigyu craigyu reopened this Aug 1, 2024
Copy link
Contributor

@RMCampos RMCampos left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@craigyu craigyu merged commit c425394 into main Aug 1, 2024
27 of 28 checks passed
@craigyu craigyu deleted the feat/1361-add-cancel-button-at-seedlot-review-page branch August 1, 2024 22:02
@craigyu craigyu linked an issue Aug 2, 2024 that may be closed by this pull request
@RMCampos RMCampos restored the feat/1361-add-cancel-button-at-seedlot-review-page branch September 13, 2024 17:50
@DerekRoberts DerekRoberts deleted the feat/1361-add-cancel-button-at-seedlot-review-page branch September 19, 2024 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
front-end front-end related task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Primary and Secondary Orchard selection Add Cancel button at seedlot review page and Client + Code Refactor
4 participants