Full page alert banner

[Adnan-cds]

Full page bundle for the Alert banner entity.

Contains config files, theme templates, and a related asset library.

This feature is totally dependent on Bootstrap making it unsuitable for general use outside the localgov_theme. The next step is to replace the Bootstrap specific code with plain Javascript and CSS.

Closes #134

FAO @andybroomfield @cjstevens78 @tom-steel @willguv

[andybroomfield]

Think you mean localgovdrupal/localgov_core which media is a submodule.
... However I'd like to reconsider if we should be adding dependencies here as this module currently stands alone. Could this be moved to a composer suggestion instead.

[Adnan-cds]

Thanks Andy.

[Adnan-cds]

localgovdrupal/localgov_core has now moved into the new suggests section as per Andy's suggestion.

[Adnan-cds]

Hi Andy,
A little clarification. This change request is more of a demo than a merge request. You may remember I mentioned in Slack that I will pass you the config files we have used for the Full page alert banner. So here they are. I don't have any preference for keeping the Full page banner functionality within the localgov_alert_banner module or in one of its submodule or somewhere else.

[Adnan-cds]

Hi Andy,
I have turned the Full page alert banner code into a submodule. Chris is working to replace the Bootstrap-specific parts with plain Javascript and CSS. Once that's done, we hope to take this change from Draft to Ready-for-review.

@andybroomfield @cjstevens78

[andybroomfield]

Thanks @Adnan-cds
@willguv @finnlewis has this change been approved by product group / working group? Following on from the slack discussions.

[Adnan-cds]

Ready for review. Finally :)

To summarize the change, it adds a submodule called localgov_alert_banner_full_page that can be enabled or disabled as needed. When enabled, it offers a bundle (i.e. variant) of the standard Alert banner that can be edited like any other Alert banner.

@andybroomfield @danchamp @finnlewis @tom-steel @stephen-cox @willguv

[andybroomfield]

My glance through at the code says this is fine.
Is there any other council that implements (or is planning on implementing) a full screen alert banner able to test and review this as I'm going to be away.
@danchamp @finnlewis (Lambeth?) or others.

[finnlewis]

@Adnan-cds I merged back from the 1.x branch to bring in the latest changes and resolve merge conflict in composer.json.

I am now testing locally and when creating a full page alert, the moderation state form element is displayed on the "Are you sure you want to set live?' page. This seems to prevent me clicking the confirm, so I think the forms are conflicting in some way.

Have you seen this?

I assume the desired behaviour is to NOT see the moderation state, in line with the default alert banner variant.

See screenshot below.

[finnlewis]

@Adnan-cds I fixed that issue with 941536a

[finnlewis]

When logged in, the full page alert is not full page, which is a bit jarring from a UX perspective.

I'm guessing with a bit of CSS tinkering we can make it behave the same logged in or logged out.

@Adnan-cds / @cjstevens78 it would be good to get this resolved this sprint, do you have time to look at this today?

And perhaps @markconroy could you take a quick look at the css / js in this pull request?

[Adnan-cds]

I fixed that issue with 941536a

Thanks Finn :)

it would be good to get this resolved this sprint, do you have time to look at this today?

I will leave it for Chris.

I like the "Big bad news" image :D

[finnlewis]

@cjstevens78 are you able to take a look at this? It would be good to get this merged today or tomorrow if at all possible.... to coincide with the end of sprint.

If not, if you can point me in the right direction.... :)

[cjstevens78]

@cjstevens78 are you able to take a look at this? It would be good to get this merged today or tomorrow if at all possible.... to coincide with the end of sprint.

If not, if you can point me in the right direction.... :)

@finnlewis I cant replicate this locally - although thinking about it I wonder if its maybe a good thing for it not to completely hijack the page when logged in because the user in this instance wants to see the site they are interacting with and aren't required to focus their attention on the pop up.

[cjstevens78]

@finnlewis we've just had a discussion about this at standup - could we get this merged in as is and look at the logged in overlay as a separate ticket?

[finnlewis]

Sure thing, happy to merge as is.

[finnlewis]

Looks good to me!

[Adnan-cds]

Thanks Finn. Merging...