-
Notifications
You must be signed in to change notification settings - Fork 17
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
Add email subscribe/unsubscribe flash messages #2271
Conversation
88b14fa
to
142715c
Compare
142715c
to
2e3e2e2
Compare
2e3e2e2
to
cf5b14b
Compare
cf5b14b
to
0e3d3c2
Compare
@alex9smith Can you rebase this so I can deploy the branch for testing? |
0e3d3c2
to
037ec4e
Compare
When a user subscribes or unsubscribes to a single page email notification topic, they'll be redirected back to the content page with a message in their account session flash. This message will be detected and used to display the relevant success message on the page. We need to set the vary header to indicate to Fastly that the success messages are different variations and should be cached and served separately. The `GovukPersonalisation::ControllerConcern` handles this for us but sets the header to vary on the full account session. This would result in pages being cached per-user which isn't very helpful in this case. Instead, override the `set_account_vary_header` method to vary on the flash message instead. The message content won't vary per user so this will mean we only cache three versions of the page - one without a flash message, one with the subscribed message and one with the unsubscribed message.
037ec4e
to
16ffe45
Compare
16ffe45
to
576eb9c
Compare
@barrucadu should be good now |
When a user completes the single page email subscribe or unsubscribe journey they will be redirected to the page they were on with a message in the account session flash. Check for these messages in the flash and render the appropriate success banner. The pages chosen for phase 1 and phase 2 of the rollout are either `publication` or `detailed_guide` content types, so only add the partial to these templates. In the future when we roll out to more pages we can add the partial to the rest of the content types.
576eb9c
to
0c88d6f
Compare
I've fixed most of the spacing, column width and content changes requested from design review. There is one outstanding which we are deciding to leave for now as it won't be a simple fix. On desktop only, there is a lot of extra spacing underneath the flash message banners: This does not appear on mobile. To fix, we'll probably need to modify the component so it can take a custom |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me (well, other than the extra spacing, but that's a separate issue)
When a user completes the single page email subscribe or unsubscribe
journey they will be redirected to the page they were on with a message
in the account session flash.
Check for these messages in the flash and render the appropriate success
banner. The pages chosen for phase 1 and phase 2 of the rollout are
either
publication
ordetailed_guide
content types, so only add thepartial to these templates. In the future when we roll out to more pages
we can add the partial to the rest of the content types.
Also set the vary header to indicate to Fastly that the success
messages are different variations and should be cached and served
separately. The
GovukPersonalisation::ControllerConcern
handles thisfor us but sets the header to vary on the full account session. This
would result in pages being cached per-user which isn't very helpful in
this case.
Instead, override the
set_account_vary_header
method to vary on theflash message instead. The message content won't vary per user so this
will mean we only cache three versions of the page - one without a flash
message, one with the subscribed message and one with the unsubscribed
message.
Trello
Subscribe:
Unsubscribe: