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: 1773 milestone calculated and adjusted fields #1805

Merged
merged 11 commits into from
Jul 24, 2023
Merged

Fix: 1773 milestone calculated and adjusted fields #1805

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ef015d5
fix: add milestone calculated values to form context
Sepehr-Sobhani Jul 18, 2023
39d7c76
test: add milestone summary report form regression test for calculate…
Sepehr-Sobhani Jul 18, 2023
113b773
chore: move mocked data into corresponding tests
Sepehr-Sobhani Jul 18, 2023
ca682c1
chore: styling tweaks to `ReadOnlyAdjustableCalculatedValueWidget`
Sepehr-Sobhani Jul 18, 2023
1ab16bd
test: update snapshots
Sepehr-Sobhani Jul 18, 2023
58929b9
chore: show default message when the calculated values aren't calcula…
Sepehr-Sobhani Jul 21, 2023
5fbcabd
chore: tweaking readonly adjustable calculated calue widget style
Sepehr-Sobhani Jul 21, 2023
1e259aa
chore: update unit test snapshots
Sepehr-Sobhani Jul 21, 2023
c15ab03
chore: remove jest snapshots
Sepehr-Sobhani Jul 21, 2023
d3e164d
test: fix e2e test
Sepehr-Sobhani Jul 21, 2023
728f1fc
chore: implement review suggestions
Sepehr-Sobhani Jul 21, 2023
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
6 changes: 6 additions & 0 deletions app/components/Form/ProjectEmissionIntensityReportFormSummary.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,12 @@ const ProjectEmissionsIntensityReportFormSummary: React.FC<Props> = ({
isOnAmendmentsAndOtherRevisionsPage,
}}
/>
<style jsx>{`
:global(.adjustable-calculated-value-widget) {
display: flex;
flex-wrap: wrap;
}
`}</style>
</>
);
};
Expand Down
11 changes: 10 additions & 1 deletion app/components/Form/ProjectMilestoneReportFormSummary.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,12 @@ const ProjectMilestoneReportFormSummary: React.FC<Props> = ({
uiSchema={updatedUiSchema}
formData={milestoneFormDiffObject.formData}
formContext={{
calculatedHoldbackAmount:
milestoneReport?.newFormData?.calculatedHoldbackAmount,
calculatedGrossAmount:
milestoneReport?.newFormData?.calculatedGrossAmount,
calculatedNetAmount:
milestoneReport?.newFormData?.calculatedNetAmount,
operation: milestoneReport.operation,
oldData:
milestoneReport.formChangeByPreviousFormChangeId?.newFormData,
Expand All @@ -244,11 +250,14 @@ const ProjectMilestoneReportFormSummary: React.FC<Props> = ({
isOnAmendmentsAndOtherRevisionsPage,
}}
/>

<style jsx>{`
div.reportContainer {
padding-top: 1em;
}
:global(.adjustable-calculated-value-widget) {
display: flex;
flex-wrap: wrap;
}
`}</style>
</div>
);
Expand Down
8 changes: 4 additions & 4 deletions app/cypress/e2e/cif/project-revision/create-first-project-revision.cy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,10 +266,10 @@ describe("when creating a project, the project page", () => {
// GHG performance uses the ReadOnlyAdjustableCalculatedValueWidget, which has a different HTML structure than the other fields
cy.findByText("GHG Emission Intensity Performance")
.next()
.should(
"have.text",
"200.00%GHG Emission Intensity Performance (Adjusted)100%"
);
.should("have.text", "200.00%");
cy.findByText("GHG Emission Intensity Performance (Adjusted)")
.next()
.should("have.text", "100%");
cy.findByText("Payment Percentage of Performance Milestone Amount (%)")
.next()
.should("have.text", "100.00%");
Expand Down
1 change: 1 addition & 0 deletions app/data/jsonSchemaForm/emissionIntensityUiSchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export const emissionIntensityReportingRequirementUiSchema = {
"ui:tooltip": {
text: emissionsIntentityTooltips.adjustedEmissionsIntensityPerformance,
},
"ui:classNames": "adjustable-calculated-value-widget",
},
calculatedEiPerformance: {
isPercentage: true,
Expand Down
3 changes: 3 additions & 0 deletions app/data/jsonSchemaForm/projectMilestoneUiSchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ const projectMilestoneUiSchema = {
isMoney: true,
hideOptional: true,
calculatedValueFormContextProperty: "calculatedGrossAmount",
"ui:classNames": "adjustable-calculated-value-widget",
},
adjustedNetAmount: {
"ui:widget": "AdjustableCalculatedValueWidget",
Expand All @@ -99,6 +100,7 @@ const projectMilestoneUiSchema = {
isMoney: true,
hideOptional: true,
calculatedValueFormContextProperty: "calculatedNetAmount",
"ui:classNames": "adjustable-calculated-value-widget",
},
adjustedHoldbackAmount: {
"ui:widget": "AdjustableCalculatedValueWidget",
Expand All @@ -108,6 +110,7 @@ const projectMilestoneUiSchema = {
isMoney: true,
hideOptional: true,
calculatedValueFormContextProperty: "calculatedHoldbackAmount",
"ui:classNames": "adjustable-calculated-value-widget",
},
dateSentToCsnr: {
"ui:widget": "DateWidget",
Expand Down
2 changes: 1 addition & 1 deletion app/lib/theme/ReadOnlyFieldTemplate.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const FieldTemplate: React.FC<FieldTemplateProps> = ({
}) => {
return (
<div>
<div className="definition-container">
<div className={`${uiSchema["ui:classNames"] ?? "definition-container"}`}>
{displayLabel && (
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a great solution, nice work! I think it could be simplified to something like:

```<div className={ uiSchema.calculatedValueFormContextProperty ? "adjustable-calculated-value-widget":"definition-container"}`}>

Then any calculated field will get the style without us having to remember to add classnames to the uiSchema

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great suggestion 🤩

<FieldLabel
label={label}
Expand Down
49 changes: 14 additions & 35 deletions app/lib/theme/widgets/ReadOnlyAdjustableCalculatedValueWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { WidgetProps } from "@rjsf/core";
import NumberFormat from "react-number-format";
import CalculatedValueWidget from "./CalculatedValueWidget";

const ReadOnlyAdjustableCalculatedValueWidget: React.FC<WidgetProps> = (
props
Expand All @@ -10,34 +11,14 @@ const ReadOnlyAdjustableCalculatedValueWidget: React.FC<WidgetProps> = (
const isMoney = uiSchema?.isMoney;
const isPercentage = uiSchema?.isPercentage;

// If we need to set the amount of decimal places, we can set it in the uiSchema, otherwise there will be no decimal places.
const numberOfDecimalPlaces = isMoney
? 2
: uiSchema?.numberOfDecimalPlaces ?? 0;

const calculatedValue =
formContext[uiSchema.calculatedValueFormContextProperty];

const adjustedInputId = `${id}_adjusted`;

return (
<div>
{calculatedValue && (
<>
{
<NumberFormat
thousandSeparator
fixedDecimalScale={numberOfDecimalPlaces}
decimalScale={numberOfDecimalPlaces}
id={id}
prefix={isMoney ? "$" : ""}
suffix={isPercentage ? "%" : ""}
value={calculatedValue}
displayType="text"
/>
}
</>
)}
<>
<CalculatedValueWidget {...props} />
{value !== calculatedValue && (
<div className={"adjustedValue"}>
<dt>{label} (Adjusted)</dt>
Expand All @@ -57,21 +38,19 @@ const ReadOnlyAdjustableCalculatedValueWidget: React.FC<WidgetProps> = (
) : (
<em>Not added</em>
)}
<style jsx>{`
.adjustedValue {
display: flex;
flex-basis: 100%;
padding-top: 0.5em;
Comment on lines +42 to +45
Copy link
Collaborator

Choose a reason for hiding this comment

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

Overall the new styling is a huge improvement so I'd be comfortable approving this as-is, but I did come across some extra spacing when the adjusted value is filled out if you want to keep tweaking:
Screenshot from 2023-07-21 15-43-34
Screenshot from 2023-07-21 15-47-21

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}
dt {
margin: 0 1em 0 0;
}
`}</style>
</div>
)}

<style jsx>{`
div.adjustedValue {
display: flex;
}
div.better {
display: flex;
}
dt {
margin: 0 1em 0 1em;
}
`}</style>
</div>
</>
);
};

Expand Down
5 changes: 1 addition & 4 deletions app/tests/unit/components/Form/ProjectEmissionIntensityReportFormSummary.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ describe("the emission intensity report form component", () => {
},
};
componentTestingHelper.loadQuery(customPayload);
const component = componentTestingHelper.renderComponent();
componentTestingHelper.renderComponent();

// Note the exclusion of the GHG Emission Intensity Performance (Adjusted) field here
// This is captured in AdjustableCalculatedValueWidget.test.tsx
Expand All @@ -370,8 +370,5 @@ describe("the emission intensity report form component", () => {
name: "actual-performance-milestone-amount-tooltip",
})
).toBeInTheDocument();

// check that tooltip text hasn't changed
expect(component.container).toMatchSnapshot();
});
});
9 changes: 2 additions & 7 deletions app/tests/unit/components/Form/ProjectFundingAgreementFormSummary.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@ describe("The Project Funding Agreement Form Summary", () => {
};
},
});
const component = componentTestingHelper.renderComponent();
componentTestingHelper.renderComponent();
expect(
screen.getAllByRole("tooltip", {
name: "maximum-funding-amount-tooltip",
Expand Down Expand Up @@ -795,9 +795,6 @@ describe("The Project Funding Agreement Form Summary", () => {
name: "total-eligible-expenses-to-date-tooltip",
})
).toHaveLength(1);

// check that tooltip text hasn't changed
expect(component.container).toMatchSnapshot();
});

it("renders the tooltips for the IA form", () => {
Expand Down Expand Up @@ -880,7 +877,7 @@ describe("The Project Funding Agreement Form Summary", () => {
};
},
});
const component = componentTestingHelper.renderComponent();
componentTestingHelper.renderComponent();

expect(
screen.getAllByRole("tooltip", {
Expand Down Expand Up @@ -913,7 +910,5 @@ describe("The Project Funding Agreement Form Summary", () => {
name: "additional-funding-amount-tooltip",
})
).toHaveLength(1);
// check that tooltip text hasn't changed
expect(component.container).toMatchSnapshot();
});
});
Loading
Loading