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

chore: update Confirmation banner description to use rich text & reword details #3545

Merged
merged 2 commits into from
Aug 22, 2024

Conversation

jessicamcinchak
Copy link
Member

@jessicamcinchak jessicamcinchak commented Aug 21, 2024

Changes:

  • Updates banner description to be a rich text input instead of plain text
  • Refactors details to be handled directly in the Confirmation component rather than at the Node level
    • There was an original request to make the "details" property names customisable while keeping the existing values, but after chatting with August we decided against this approach
    • We don't let editors change the text of other static pages (eg Application Saved) or Gov Notify email templates, and we ultimately want to ensure all these fields have consistent naming between these various places - so customisation in this one place doesn't feel like the right solution and would likely lead to Confirmation page language that is out of sync/disjointed from other parts of the service
    • We did however update the existing language to be slightly more generic and accurate - eg Planning application referenceApplication reference (now matches emails), SubmittedPaid at (matches actual data 🙈)

Screenshot from 2024-08-21 19-56-09

Follow-ups:

  • I'll create a new Trello ticket to pick back up the larger discussion of if we want to drop "application" language throughout, everything we need to touch to do that fully, and how we might achieve it (eg do we just agree on another word or read dynamically from schema application types where possible etc) - see thread here: https://opensystemslab.slack.com/archives/C07F7215W7P/p1724253793464279

@jessicamcinchak
Copy link
Member Author

);
}

// TODO - Retire in favor of ODP Schema application type descriptions or fallback to flowName
Copy link
Member Author

Choose a reason for hiding this comment

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

Don't want to do this yet because we know our ODP Schema enums are a bit in flux, but imagine it will be the better / more consistent way forward ! This function only does special handling for LDCs - which is fine for now as they're the only live statutory service.

@jessicamcinchak jessicamcinchak marked this pull request as ready for review August 21, 2024 18:20
@jessicamcinchak jessicamcinchak requested a review from a team August 21, 2024 18:20
Copy link

github-actions bot commented Aug 21, 2024

Removed vultr server and associated DNS entries

data: QuestionAndResponses[];
}

export function Presentational(props: PresentationalProps) {
Copy link
Member Author

Choose a reason for hiding this comment

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

The addition of a Presentational component here is purely a convenience so we can continue mocking applicableDetails for Storybook (eg similar to Review page which is another component that relies heavily on store data). Could just as easily return this via Confirmation directly, but then Storybook would only ever display a single row "Application reference" for details section as that's gauranteed to always be in state.

Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

One very small comment, but all looking good. The move to just more generic language is a good one I think, thanks for cross-checking with @augustlindemer on this one 👌

Always nice to see some quality tidy ups as well!

Comment on lines +58 to +65
color: props.node?.color || {
text: "#000",
background: "rgba(1, 99, 96, 0.1)",
},
heading: props.node?.data?.heading || "Application sent",
description:
props.node?.data?.description ||
`A payment receipt has been emailed to you. You will also receive an email to confirm when your application has been received.`,
`<p>A payment receipt has been emailed to you. You will also receive an email to confirm when your application has been received.</p>`,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the standard pattern we're using here is to put the default values into the parse<COMPONENT_TYPE>() function in the associated model.ts - a little cleaner and more consistent?

@jessicamcinchak jessicamcinchak merged commit c9d5939 into main Aug 22, 2024
12 checks passed
@jessicamcinchak jessicamcinchak deleted the jess/confirmation-rich-text-refactor branch August 22, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants