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

Support hero image sizes for landing pages #9579

Merged
merged 3 commits into from
Nov 19, 2024

Conversation

richardTowers
Copy link
Contributor

@richardTowers richardTowers commented Nov 5, 2024

This allows users to add various hero image sizes to LandingPage editions (but no other types of edition).

The user interface looks like this:

Screenshot of Whitehall showing the interface for adding images. There are radio buttons for "What kind of image is this?"

The markdown snippets for these images can be included in the blocks YAML on the landing page, and the recursively_expand_images method will replace them with image details (including the uploaded variants), irrespective of how deep they appear in the structure.

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

@richardTowers richardTowers force-pushed the different-image-sizes-landing-pages branch from b852bd7 to f3f6fe7 Compare November 6, 2024 11:09
@richardTowers richardTowers force-pushed the different-image-sizes-landing-pages branch from 46ca634 to ca62d8c Compare November 6, 2024 15:32
@richardTowers richardTowers force-pushed the different-image-sizes-landing-pages branch from ca62d8c to 251618e Compare November 6, 2024 15:33
@richardTowers richardTowers force-pushed the different-image-sizes-landing-pages branch from 251618e to 1fecd90 Compare November 6, 2024 16:35
@richardTowers richardTowers force-pushed the different-image-sizes-landing-pages branch from 1fecd90 to 457869b Compare November 6, 2024 16:37
@richardTowers richardTowers force-pushed the different-image-sizes-landing-pages branch from 457869b to ca937f6 Compare November 6, 2024 16:53
@richardTowers richardTowers force-pushed the different-image-sizes-landing-pages branch 2 times, most recently from cca168b to a4f0554 Compare November 6, 2024 17:19
@richardTowers richardTowers changed the title Different image sizes landing pages Support hero image sizes for landing pages Nov 6, 2024
@richardTowers richardTowers force-pushed the different-image-sizes-landing-pages branch 5 times, most recently from c991530 to a35c116 Compare November 8, 2024 11:59
@richardTowers richardTowers force-pushed the different-image-sizes-landing-pages branch from 2fba5df to c2c6d3c Compare November 8, 2024 14:58
@richardTowers richardTowers force-pushed the different-image-sizes branch 2 times, most recently from 60d8cc4 to e0ade76 Compare November 14, 2024 13:50
@richardTowers richardTowers force-pushed the different-image-sizes-landing-pages branch 2 times, most recently from daec668 to f7e9774 Compare November 14, 2024 18:02
Base automatically changed from different-image-sizes to main November 18, 2024 10:59
@richardTowers richardTowers marked this pull request as ready for review November 18, 2024 11:25
@richardTowers
Copy link
Contributor Author

Example landing page with images published using this branch (for those with access to integration):

https://draft-origin.integration.publishing.service.gov.uk/landing-page/rory

@richardTowers richardTowers force-pushed the different-image-sizes-landing-pages branch from f7e9774 to af572ca Compare November 18, 2024 15:47
Copy link
Contributor

@KludgeKML KludgeKML left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@ryanb-gds ryanb-gds left a comment

Choose a reason for hiding this comment

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

I don't know if there's really time to do anything about this, but personally I would like to see each block type have a (non-Active Record-backed) model class responsible for the logic of validation (could use ActiveModel validation) and mapping image tags to URLs. On the assumption that we're not in a position to start trying to nail down a well-designed object model just yet I'm approving.

Also, outside the scope of this PR, but I hadn't seen the extending of other landing pages' YAML body before and now I won't sleep tonight 😅

@richardTowers
Copy link
Contributor Author

Yeah, I'm reluctant to take too many steps down the "model the blocks properly" path, because I'm really still not sure that Whitehall is the right place for this kind of publishing at all. Maybe it is (in which case we should model things properly), or maybe it isn't (in which case we should keep it as simple as possible, and then excise this code as soon as possible).

It would be nice to have some activemodel validations on the blocks, so we could tell the user about errors rather than putting the errors in the content item (like I'm doing here) though.

I think I will also need to expand images in "featured" blocks in a future PR though, so I'll have a think about whether it's better to introduce a more proper model at that point.

@richardTowers richardTowers merged commit ed78c0e into main Nov 19, 2024
19 checks passed
@richardTowers richardTowers deleted the different-image-sizes-landing-pages branch November 19, 2024 09:38
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.

3 participants