feat: conditionally show/hide "Continue" button on Confirmation using `isFinalCard` #4029

jessicamcinchak · 2024-12-02T16:59:36Z

Per request here: https://opensystemslab.slack.com/archives/C01E3AC0C03/p1733157241528129

Changes:

The Confirmation component now determines if it is the final node in the flow
- If it is the final node, it will not display a "Continue" button
- If it is not the final node, it will display a "Continue" button and leave a basic breadcrumb
- In either scenario, the Confirmation component still never displays a "back" button - which I think is correct/intended behavior as we would not allow someone to change answers post-Confirmation
Hides the "Save and return..." link beneath "Continue" if there's a Send component present in our breadcrumbs

Todo:

✅ Add some tests (honestly a little unsettling that this hasn't broken any 😅)

Testing questions:

Does this work as expected? Is there any sluggishness introduced when navigating between cards?
Is it okay that any node that comes after Confirmation will not be included in a submission payload? Does this impact any of our existing feedback/analytics processes for tracking user satisfaction, etc?
Do we need to hide the "Save and resume this application later" link under "Continue" on all nodes after Confirmation?
- ✅ See note above, now hidden if breadcrumbs include a Send node
Do we need to introduce any publish validation rules for types of components that are allowed to come after Confirmation, eg no Pay or Send?

…ng isFinalCard

jessicamcinchak · 2024-12-02T17:01:48Z

editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts

-    // OSL Slack explanation: https://bit.ly/3x38IRY
-    return false;
+    const { upcomingCardIds } = get();
+    return upcomingCardIds().length === 1;


This one's been a long time coming (original commit) & more promising to re-introduce now since upcomingCardIds has been recently refactored to only be responsible for navigation and no longer optimistic auto-answering

Very happy to see this commented back in 👍

jessicamcinchak · 2024-12-02T17:04:56Z

editor.planx.uk/src/pages/Preview/Node.tsx

+    state.cachedBreadcrumbs,
+  ]);
+
+  const handleSubmit = props.handleSubmit;


Ideally, in the near future, we can/will re-introduce isFinalCard at the Preview/Node level, but we'll want to do more thorough performance reviews and add test coverage before doing this - so have adjusted this to keep current behavior in the meantime and instead only reference isFinalCard within the Confirmation component itself rather than any component (which should be significantly less expensive / less risky to introduce slugishness)

Totally agree - this is the right approach here 👍

github-actions · 2024-12-02T17:22:23Z

Removed vultr server and associated DNS entries

DafyddLlyr

PR description super helpful thanks. Great point about tests - would be super nice to see them added 👍

The question around validation checks is a good one -

We probably don't want anything that sets a passport value coming after send (maybe this just needs to be an alert, people may want to do something after send around next steps etc)
We can't have Feedback be the final component of a service, as it only inserts a feedback record when "Continue" is hit

DafyddLlyr · 2024-12-03T09:13:06Z

editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx

+  // Check if we have a Send node in our breadcrumbs
+  //   This is a better/more immediate proxy for "Submitted" because actual send events that populate lowcal_sessions.submitted_at are queued via Hasura
+  const hasSent = Object.keys(breadcrumbs)
+    .reverse()


nit: As breadcrumb keys are sorted alphabetically not chronologically this doesn't offer any benefits (despite being a clever and nice optimisation!)

ah shoot yes always forget that alphabetical key storage ! I'll remove the .reverse() here and just call .some() directly (at least it stops at first find)

Very much assuming it's NOT worth the expense of sorting, then reversing, then calling .some() - but maybe one to revisit in future

Yep - there's a small number of keys here so I think it's also not worth the sorting step really 👍

DafyddLlyr · 2024-12-03T09:14:41Z

editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts

-    // OSL Slack explanation: https://bit.ly/3x38IRY
-    return false;
+    const { upcomingCardIds } = get();
+    return upcomingCardIds().length === 1;


Very happy to see this commented back in 👍

DafyddLlyr · 2024-12-03T09:15:51Z

editor.planx.uk/src/pages/Preview/Node.tsx

+    state.cachedBreadcrumbs,
+  ]);
+
+  const handleSubmit = props.handleSubmit;


Totally agree - this is the right approach here 👍

jessicamcinchak · 2024-12-03T12:43:16Z

Allow Confirmation components to conditionally have a "Continue" button so that Feedback can be collected post-Confirmation

conditionally show/hide continue button on Confirmation component usi…

a5ce539

…ng isFinalCard

jessicamcinchak commented Dec 2, 2024

View reviewed changes

jessicamcinchak added 3 commits December 3, 2024 08:44

never show SaveAndReturnButton on Confirmation nodes

62bcbc6

hide SaveAndReturnButton if breadcrumbs have Send

6f4fcf1

clearer code comment

9afebe9

jessicamcinchak marked this pull request as ready for review December 3, 2024 08:55

jessicamcinchak requested a review from a team December 3, 2024 08:55

DafyddLlyr approved these changes Dec 3, 2024

View reviewed changes

jessicamcinchak added 3 commits December 3, 2024 11:36

don't reverse(), add tests

5f44a70

more tests

ca4e714

correct test syntax

51a7ec3

jessicamcinchak merged commit 5f79efc into main Dec 3, 2024
12 checks passed

jessicamcinchak deleted the jess/confirmation-continue-button branch December 3, 2024 11:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: conditionally show/hide "Continue" button on Confirmation using `isFinalCard` #4029

feat: conditionally show/hide "Continue" button on Confirmation using `isFinalCard` #4029

jessicamcinchak commented Dec 2, 2024 •

edited

Loading

jessicamcinchak Dec 2, 2024 •

edited

Loading

DafyddLlyr Dec 3, 2024

jessicamcinchak Dec 2, 2024 •

edited

Loading

DafyddLlyr Dec 3, 2024

github-actions bot commented Dec 2, 2024 •

edited

Loading

DafyddLlyr left a comment

DafyddLlyr Dec 3, 2024

jessicamcinchak Dec 3, 2024

DafyddLlyr Dec 3, 2024

DafyddLlyr Dec 3, 2024

DafyddLlyr Dec 3, 2024

jessicamcinchak commented Dec 3, 2024

feat: conditionally show/hide "Continue" button on Confirmation using isFinalCard #4029

feat: conditionally show/hide "Continue" button on Confirmation using isFinalCard #4029

Conversation

jessicamcinchak commented Dec 2, 2024 • edited Loading

jessicamcinchak Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

DafyddLlyr Dec 3, 2024

Choose a reason for hiding this comment

jessicamcinchak Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

DafyddLlyr Dec 3, 2024

Choose a reason for hiding this comment

github-actions bot commented Dec 2, 2024 • edited Loading

DafyddLlyr left a comment

Choose a reason for hiding this comment

DafyddLlyr Dec 3, 2024

Choose a reason for hiding this comment

jessicamcinchak Dec 3, 2024

Choose a reason for hiding this comment

DafyddLlyr Dec 3, 2024

Choose a reason for hiding this comment

DafyddLlyr Dec 3, 2024

Choose a reason for hiding this comment

DafyddLlyr Dec 3, 2024

Choose a reason for hiding this comment

jessicamcinchak commented Dec 3, 2024

feat: conditionally show/hide "Continue" button on Confirmation using `isFinalCard` #4029

feat: conditionally show/hide "Continue" button on Confirmation using `isFinalCard` #4029

jessicamcinchak commented Dec 2, 2024 •

edited

Loading

jessicamcinchak Dec 2, 2024 •

edited

Loading

jessicamcinchak Dec 2, 2024 •

edited

Loading

github-actions bot commented Dec 2, 2024 •

edited

Loading