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

Add a freestyle crop, image styles and responsive image style, image… #145

Merged
merged 1 commit into from
Sep 26, 2022

Conversation

msayoung
Copy link
Member

… view mode

@msayoung msayoung changed the title Add a freestyle crop, image styles and responsive image style, impage… Add a freestyle crop, image styles and responsive image style, image… Sep 14, 2022
@msayoung msayoung marked this pull request as ready for review September 16, 2022 11:58
Copy link
Member

@finnlewis finnlewis left a comment

Choose a reason for hiding this comment

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

Tested locally and works for me on a fresh install with localgov_demo enabled.

This will only affect fresh installs too.

Thanks @msayoung !

Copy link
Member

@markconroy markconroy left a comment

Choose a reason for hiding this comment

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

Checked on microsites and this does what it says on the tin, however I'm wary of the responsive freestyle image style. Just because someone is on a large screen does not mean they will want each freestyle image to be 1200px wide. For example, embedding a headshot of a councillor. you might just want it to be 300px wide.

Is this a concern? If not, I'm happy to approve.

@msayoung
Copy link
Member Author

It's true - Do you have any thoughts on how we deal with this @markconroy ?

@finnlewis
Copy link
Member

From our discussion on the Merge Monday call, I think we're good to go with this @msayoung

We noted we would follow up on another issue with @markconroy to explore how to give content designers the info they need.

@msayoung msayoung merged commit 198272b into 2.x Sep 26, 2022
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