CCM-16455: Add warning banner to "Create a message plan" page about editing a campaign#913
CCM-16455: Add warning banner to "Create a message plan" page about editing a campaign#913
Conversation
andykay-nhs
force-pushed
the
feature/CCM-16455-add-create-message-plan-warning
branch
5 times, most recently
from
7499e0a to
aa915fb
Compare
| initialState.name | ||
| ); | ||
| const [campaignId, handleCampaignIdChange] = useTextInput<HTMLSelectElement>( | ||
| const [_, handleCampaignIdChange] = useTextInput<HTMLSelectElement>( |
There was a problem hiding this comment.
if you're not using campaignId for anything, why do you need handleCampaignIdChange at all?
There was a problem hiding this comment.
thats an excellent point! I think I had assumed that there was some server action form magic going on that was still utilising this to an extent? but I will look into it to be sure and report back
There was a problem hiding this comment.
yep doesn't seem to be needed 🤷 removed now
andykay-nhs
force-pushed
the
feature/CCM-16455-add-create-message-plan-warning
branch
6 times, most recently
from
56c77fc to
9bef55a
Compare
| class="nhsuk-warning-callout nhsuk-u-margin-bottom-5 nhsuk-u-margin-top-5" | ||
| data-testid="campaign-warning-callout" | ||
| > | ||
| <h3 |
There was a problem hiding this comment.
might have to run it through an a11y tool to confirm but is it weird that this is an <h3> in the context of the structure of the page?
There was a problem hiding this comment.
there were some complaints about skipping from h1 -> h3 so have updated the callout header to be an h2 now. It still seems a bit weird but the extension I was using is no longer complaining at least 🤷. Using the NHS react component for this locks you in to using a heading element so the alternative is to ditch that and make the heading using regular html + nhs css classes. Is there a specific tool you'd recommend using to check this?
There was a problem hiding this comment.
yeah it feels like it shouldnt be a heading to me, maybe one to confirm with Reddy/Emma
and if they agree, presumably its just a change to use NHSNotifyWarningCallout instead then we can have more granular control over the markup?
andykay-nhs
force-pushed
the
feature/CCM-16455-add-create-message-plan-warning
branch
from
9bef55a to
41df2ea
Compare
5 participants
Description
(created on Andy's behalf).
single campaign
multiple campaigns
form error
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.