Skip to content

Fix image block alignment - #8262

Closed
iRohitSingh wants to merge 2 commits into
sevenfrom
image-alignment
Closed

Fix image block alignment#8262
iRohitSingh wants to merge 2 commits into
sevenfrom
image-alignment

Conversation

@iRohitSingh

@iRohitSingh iRohitSingh commented May 21, 2026

Copy link
Copy Markdown
Contributor

Related pr: #8261

@iRohitSingh
iRohitSingh requested a review from TimoBroeskamp May 21, 2026 07:16
@davisagli davisagli added this to the Seven milestone May 21, 2026
max-width: 50%;
margin-right: 1em !important;
margin-bottom: 1em;
float: left;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@iRohitSingh Why is this using float for alignment?

Can we instead use a modern layout approach (flexbox, grid, or display: flow-root

max-width: 50%;
margin-bottom: 1em;
margin-left: 1em !important;
float: right;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The

element with float requires clear: both on the parent.

Let's try using a more modern flex box / grid approach.

import Image from '@plone/layout/components/Image/Image';
import clsx from 'clsx';
import config from '@plone/registry';
import './ImageBlock.css';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Aren't we supposed to use tailwind in Seven?


return (
<div
<figure

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should re-use ImageView component in the Edit component

large: data.size === 'l',
medium: data.size === 'm',
small: data.size === 's',
})}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should also have

loading="lazy"
responsive={true}

>
<Image
className={clsx({
'full-width': data.align === 'full',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add full to the alignment actions

@iFlameing

Copy link
Copy Markdown
Member

@avoinea sorry, you have to review this pr. @iRohitSingh and @TimoBroeskamp worked on same thing in sprint. We are going in favour of Timo B pr because there we are using new css rule for all the floated blocs. One css for all of them. Here is the pr if you want to review it.

PR: #8261

@iFlameing iFlameing closed this Jun 2, 2026
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.

4 participants