MPDX-9204 - Edit mode/View mode functionality, removes /edit route in favor of dynamic id route #1604

zweatshirt · 2026-01-30T20:45:48Z

Description

View mode should lead to the summary page, edit mode should lead to step one.
View mode can be assumed to be !isEdit, I didn't see useful reason to use dedicated view logic.
Routing now uses the calculation id.
Jira ticket

Setting complete to be !isEdit is useStepList is not needed since the steps list won't render in view mode, so I can remove that if needed.

Testing

Go to the Pending Landing of Salary Calculator
Click the View button and ensure it only takes you to the Summary and the side panel is not displayed.
Check that the Edit button takes you to the beginning of the form and allows editing.

Checklist:

I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
I have applied the appropriate labels (Add the label "Preview" to automatically create a preview environment)
I have run the Claude Code /pr-review command locally and fixed any relevant suggestions
I have requested a review from another person on the project
I have tested my changes in preview or in staging
I have cleaned up my commit history

… routing

…and improve routing

…alculation and simplify calculation handling

…ng and routing

…er rendering of components and buttons

github-actions · 2026-01-30T20:49:06Z

Bundle sizes [mpdx-react]

Compared against 5939d0e

Route	Size (gzipped)	Diff
`/accountLists/[accountListId]/reports/salaryCalculator/[calculationId]`	159.74 KB	added
`/accountLists/[accountListId]/reports/salaryCalculator/edit`	no change	removed

github-actions · 2026-01-30T20:49:23Z

Preview branch generated at https://MPDX-9204-updated.d3dytjb8adxkk5.amplifyapp.com

…t navigation, may be improved

canac

Looks really good! Just a couple thoughts on ways you could consider refactoring the viewing vs editing logic.

...mponents/Reports/SalaryCalculator/Landing/NewSalaryCalculationLanding/LandingTestWrapper.tsx

.../Reports/SalaryCalculator/Landing/NewSalaryCalculationLanding/NewSalaryCalculatorLanding.tsx

src/components/Reports/SalaryCalculator/Landing/useLandingData.ts

src/components/Reports/SalaryCalculator/SalaryCalculatorContext/SalaryCalculatorContext.tsx

canac · 2026-02-02T15:32:40Z

...alaryCalculator/Landing/PendingSalaryCalculationLanding/components/PendingRequestActions.tsx

+    }
+    router.push({
+      pathname: `/accountLists/${accountListId}/reports/salaryCalculator/${calculation.id}`,
+      query: { mode: 'edit' },


The way we have this now, the user could add mode=edit to the URL (or go back to a page in their history) and the UI would let them try to edit a pending or approved request (although the server would block updates changes). I think we should flip it and set mode=view when the user wants to view an action-required request instead of editing it.

Then the context can set the editing flag to "(status is in-progress OR status is action-required) AND mode is not view".

src/components/Reports/SalaryCalculator/StepNavigation/StepNavigation.tsx

canac · 2026-02-02T15:35:57Z

src/components/Reports/SalaryCalculator/SalaryCalculator.tsx


  const iconPanelItems = useIconPanelItems(isDrawerOpen, toggleDrawer);

  return (


Would it work to change MainContent to render the view step instead of CurrentStep if we are in view-only mode? Then I think you might not need the changes to useStepList. I haven't tried it though - maybe that doesn't work.

…eadability

…nContent based on editing

zweatshirt · 2026-02-02T21:17:38Z

...Calculator/Landing/PendingSalaryCalculationLanding/components/PendingRequestActions.test.tsx

 }) => (
-  <LandingTestWrapper hasLatestCalculation onCall={onCall}>
-    <PendingRequestActions calculation={calculation} />
+  <LandingTestWrapper hasLatestCalculation onCall={onCall} router={router}>


router={router}>
Realized I forgot to remove this from LandingTestWrapper here, I'll do that (just can't push without using --no-verify at the moment and I'd rather not do that if I can help it)

canac

Great work implementing my suggestions! Once staging is back up, I'd like to test the UI. But thanks for the updates here! Really solid work.

...mponents/Reports/SalaryCalculator/Landing/NewSalaryCalculationLanding/LandingTestWrapper.tsx

canac · 2026-02-02T21:11:58Z

src/components/Reports/SalaryCalculator/SalaryCalculatorContext/SalaryCalculatorContext.tsx

-  const calculation = calculationData?.salaryRequest ?? null;
+  const [isDrawerOpen, setDrawerOpen] = useState(false);
+
+  useEffect(() => {


Instead of an effect, you could return something like drawerOpen: editing ? true : drawerOpen from the context (I'm not sure if that conditional is exactly what you were trying to accomplish). It might be better though to remove this useEffect and handle drawerOpen where it's used in SalaryCalculator.

Here's a section in an article about the pattern: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes. The third code block (the "Best" one) is the approach I'm recommending here.

Nice article, it was a helpful reference thanks

.../Reports/SalaryCalculator/Landing/NewSalaryCalculationLanding/NewSalaryCalculatorLanding.tsx

...Calculator/Landing/PendingSalaryCalculationLanding/components/PendingRequestActions.test.tsx

src/components/Reports/SalaryCalculator/StepNavigation/StepNavigation.tsx

… state management

zweatshirt added 8 commits January 30, 2026 13:40

Change edit route to use id

4ebe032

Update SalaryCalculator and useStepList for improved edit handling

0d4844e

Enhance SalaryCalculator navigation and edit handling with mode-based…

8ceacda

… routing

Update NewSalaryCalculatorLanding to handle in-progress calculations …

e20d403

…and improve routing

Refactor NewSalaryCalculatorLanding to remove unneeded hasInProgressC…

65c440c

…alculation and simplify calculation handling

Remove unneeded view mode, !isEdit is assumed to be view only

967f2b5

Update SalaryCalculator components and tests for improved edit handli…

d01e562

…ng and routing

Enhance SalaryCalculator tests for edit and view modes, ensuring prop…

3a7d9cd

…er rendering of components and buttons

zweatshirt self-assigned this Jan 30, 2026

zweatshirt added the Preview Environment Add this label to create an Amplify Preview label Jan 30, 2026

zweatshirt marked this pull request as ready for review January 30, 2026 20:56

zweatshirt marked this pull request as draft January 30, 2026 21:02

zweatshirt added 3 commits January 30, 2026 15:08

Fix calculation ID handling in PendingRequestActions for view and edi…

33e5fa7

…t navigation, may be improved

Try to improve SalaryCalculation creation

786ca1f

Small improvement

fb8ae1b

zweatshirt marked this pull request as ready for review January 30, 2026 21:27

zweatshirt marked this pull request as draft January 30, 2026 21:37

Add specs for view and edit route push

1e2eae6

zweatshirt marked this pull request as ready for review January 30, 2026 21:57

zweatshirt requested a review from canac January 30, 2026 21:57

canac reviewed Feb 2, 2026

View reviewed changes

zweatshirt added 5 commits February 2, 2026 10:39

Improve Edit/View logic in SalaryCalculator and small refactors for r…

b2977bd

…eadability

Refactor tests to editing and check for view mode

99d1970

Revert changes to useStepList, instead handling step rendering in Mai…

5ce5d33

…nContent based on editing

Update salary calculation input structure and simplify step list usage

2a33b27

Fix mistake in useLandingData

c59ab19

zweatshirt requested a review from canac February 2, 2026 18:09

zweatshirt commented Feb 2, 2026

View reviewed changes

canac reviewed Feb 2, 2026

View reviewed changes

zweatshirt added 2 commits February 2, 2026 15:40

Refactor SalaryCalculator components to streamline routing and drawer…

680e785

… state management

Remove unused router prop from LandingTestWrapper component

a27be42


		const iconPanelItems = useIconPanelItems(isDrawerOpen, toggleDrawer);

		return (

MPDX-9204 - Edit mode/View mode functionality, removes /edit route in favor of dynamic id route #1604

Are you sure you want to change the base?

MPDX-9204 - Edit mode/View mode functionality, removes /edit route in favor of dynamic id route #1604

Uh oh!

Conversation

zweatshirt commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Checklist:

Uh oh!

github-actions bot commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bundle sizes [mpdx-react]

Uh oh!

github-actions bot commented Jan 30, 2026

Uh oh!

canac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

canac Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

canac Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

zweatshirt Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

canac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

canac Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

zweatshirt Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zweatshirt commented Jan 30, 2026 •

edited

Loading

github-actions bot commented Jan 30, 2026 •

edited

Loading

zweatshirt Feb 2, 2026 •

edited

Loading