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

Gallery: Fix column widths in gallery block. #2629

Merged
merged 1 commit into from
Sep 4, 2017
Merged

Gallery: Fix column widths in gallery block. #2629

merged 1 commit into from
Sep 4, 2017

Conversation

gonzomir
Copy link
Contributor

@gonzomir gonzomir commented Aug 31, 2017

The widths of columns have wrong calc() expressions, which causes more images to fit in a row than set in the "Columns" setting of the block.

This should fix #2465

@karmatosed
Copy link
Member

First, with this patch I don't see any difference when looking with it applied or not, for example on a gallery of 7 items:

2017-08-31 at 15 38

Is there some step I can trace to see this? Does it rely on a number or rows? I'd love to see the impact and recreate between the two. That all said, the PR doesn't break anything.

@gonzomir
Copy link
Contributor Author

Sorry for not providing more details, I thought the issue will give enough information. Anyway, here's a gallery with 20 pictures, 5 in a row (so there must be 4 rows, all images the same size):

Before:
gutenberg-gallery-before

And after:
gutenberg-gallery-after

The problem becomes more prominent with a greater number of columns, because the calc() expressions subtract greater amounts from the width of the items, while it should always be the sum of the horizontal margins (16px in this case).

@mtias
Copy link
Member

mtias commented Sep 4, 2017

This looks good, thanks!

@mtias mtias merged commit b0df41b into WordPress:master Sep 4, 2017
@mtias mtias added [Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended labels Sep 4, 2017
@nb
Copy link
Member

nb commented Sep 5, 2017

Good fix, @gonzomir – I have no idea how that mess got in :) One possible improvement would it be to extract the margins in a variable – it would make it a bit easier to follow.

@gonzomir
Copy link
Contributor Author

gonzomir commented Sep 8, 2017

If the variable can be shared between components, It'll probably allow for more consistent styles. And also the calc() expressions can be joined in a mixin that generates all the columns CSS - like "give me maximum 8 columns for this base class". I'm not sure however how such approach fits in a independent components philosophy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gallery Columns not working
4 participants