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

Query Loop Block: make elements wrap and use 'gap' instead of 'margin-right' for 'post-template' #36832

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/block-library/src/post-template/editor.scss
Expand Up @@ -4,4 +4,12 @@
margin-left: 0;
list-style: none;
}

.wp-block-post-template {
.is-root-container {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change needed? padding-left and padding-right are already set in the rule above.

Copy link
Author

Choose a reason for hiding this comment

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

yes, there is padding coming from somewhere, this is for WYSIWYG experience.

padding-left: 0;
padding-right: 0;
}

shadigaafar marked this conversation as resolved.
Show resolved Hide resolved
}
}
18 changes: 6 additions & 12 deletions packages/block-library/src/post-template/style.scss
Expand Up @@ -19,24 +19,18 @@
flex-direction: row;
display: flex;
flex-wrap: wrap;

gap: 1.25em;
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're going to switch to using gap, like @aaronrobertshaw mentions it might be worth allowing usage of the block gap CSS variable so that we can give users control over the gap between elements. Here's an example of a CSS rule we can use from the Navigation block.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, of course, this what I thought about too, but I thought I could do it in another PR.

li {
margin: 0 0 1.25em 0;
width: 100%;
flex: 1;
min-width: 250px;
margin: 0;
}

@include break-small {
li {
margin-right: 1.25em;
}

@include break-xlarge {
Copy link
Contributor

Choose a reason for hiding this comment

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

This change effectively switches off the column count for all viewport widths less than 1080px, so if someone has set their Query Loop block to a column count of 2 for example, then in narrower viewports, 3 or 4 columns will be displayed instead.

Copy link
Author

Choose a reason for hiding this comment

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

you are right here, i haven't notice this. this can be fixed. thanks.

@for $i from 2 through 6 {
&.is-flex-container.columns-#{ $i } > li {
width: calc((100% / #{ $i }) - 1.25em + (1.25em / #{ $i }));

&:nth-child( #{ $i }n ) {
margin-right: 0;
}
flex: none;
}
}
}
Expand Down