-
Notifications
You must be signed in to change notification settings - Fork 194
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
Conversation
b852bd7
to
f3f6fe7
Compare
b00b938
to
32a8e23
Compare
46ca634
to
ca62d8c
Compare
32a8e23
to
46f9040
Compare
ca62d8c
to
251618e
Compare
46f9040
to
fa41313
Compare
251618e
to
1fecd90
Compare
fa41313
to
a02bb72
Compare
1fecd90
to
457869b
Compare
a02bb72
to
7730b66
Compare
457869b
to
ca937f6
Compare
7730b66
to
d3ee291
Compare
cca168b
to
a4f0554
Compare
c991530
to
a35c116
Compare
2fba5df
to
c2c6d3c
Compare
60d8cc4
to
e0ade76
Compare
daec668
to
f7e9774
Compare
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 |
f7e9774
to
af572ca
Compare
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.
LGTM
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.
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 😅
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. |
This allows users to add various hero image sizes to LandingPage editions (but no other types of edition).
The user interface looks like 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.Follow these steps if you are doing a Rails upgrade.