Move PictureLayout to grid - #16251
frederickobrien wants to merge 10 commits into
Conversation
f33830a to
e33629d
Compare
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
e2f5162 to
d9fba56
Compare
6729bd2 to
53aad3f
Compare
JamieB-gu
left a comment
There was a problem hiding this comment.
Overall approach looks good. Comments mostly about application of CSS and the grid rules.
| headline: { | ||
| mobile: 'grid-row: 2;', | ||
| tablet: 'grid-row: 2;', | ||
| desktop: `grid-row: 2; ${grid.between('centre-column-start', 'right-column-end')};`, |
There was a problem hiding this comment.
I think you're asking this element to span both columns here, then on the element inside it you're setting a max-width to ensure it's essentially the same width as the centre column? Would it be easier to just set grid.column.centre here (which I think is the default) and remove that max-width, or am I missing something about the design?
9877fdf to
ebfdf4a
Compare
|
Please rebase this branch against
Please rebase this branch against |
5cab1a2 to
bbe6b1b
Compare
c75a9d3 to
cc995bc
Compare
fbd642f to
b92684d
Compare
2140727 to
2b18bb3
Compare
2b18bb3 to
7e10e7c
Compare
| id="maincontent" | ||
| css={[ | ||
| `margin-top: ${remSpace[3]}`, | ||
| `margin-top: ${format.design === ArticleDesign.Picture ? 0 : remSpace[3]}`, |
There was a problem hiding this comment.
This keeps parity with the Chromatic tests though I'd argue it's better for this to be consistent
Co-authored-by: Jamie B <53781962+JamieB-gu@users.noreply.github.com>
Co-authored-by: Jamie B <53781962+JamieB-gu@users.noreply.github.com>
Co-authored-by: Jamie B <53781962+JamieB-gu@users.noreply.github.com>
🚀 Image pushed to AWS ECRImage digest: 🐛 Run the image locallyThe following can be used to run the image locally: # Refer to image using the immutable digest. Find alternatives below.
IMAGE_IDENTIFIER="@sha256:533ca6852fffa2d2d05210099edd470d711c92c04d5181ac2f74093dc2e5080f"
# Refer to image using branch tag
# IMAGE_IDENTIFIER=":branch-move-picture-layout-into-grid"
# Refer to image using build tag
# IMAGE_IDENTIFIER=":build-30847"
# Refer to image via the GitHub commit SHA tag
# IMAGE_IDENTIFIER=":sha-9a177ea782c29db4f141b5774e3d7262c11e3435"
# Set environment variables for the AWS CLI
AWS_PROFILE="<A_PROFILE_FROM_JANUS>"
AWS_DEFAULT_REGION="eu-west-1"
IMAGE_ACCOUNT_ID=$(aws ssm get-parameter --name /organisation/accounts/deployTools --query "Parameter.Value" --output text)
REGISTRY="${IMAGE_ACCOUNT_ID}.dkr.ecr.${AWS_DEFAULT_REGION}.amazonaws.com"
IMAGE="${REGISTRY}/guardian/dotcom-rendering${IMAGE_IDENTIFIER}"
# Login to AWS ECR https://docs.aws.amazon.com/AmazonECR/latest/userguide/registry_auth.html
aws ecr get-login-password | docker login --username AWS --password-stdin $REGISTRY
# Pull the image
docker pull $IMAGE
# Run the image. You'll likely need to set additional flags. See https://docs.docker.com/reference/cli/docker/container/run.
docker run $IMAGE |





Continuing on my grid rampage of #1542, #16119, and #16133 this absorbs PictureLayout into the new grid system for content pages. This one's a smidge more involved due to the avatar pics that sometimes render (though cracking that here paves the way for CommentLayout to move over too).
Screenshots