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

Margin incorrect at narrow viewport widths #275

Closed
Toutouwai opened this issue May 3, 2018 · 6 comments · Fixed by #286
Closed

Margin incorrect at narrow viewport widths #275

Toutouwai opened this issue May 3, 2018 · 6 comments · Fixed by #286

Comments

@Toutouwai
Copy link

Toutouwai commented May 3, 2018

There seems to be an issue where margins are not consistent at narrow viewport widths. Images with a wide aspect ratio in particular seem to be affected.

You can observe the issue in the Justified Gallery demo. In the screencast below note how the top margin becomes incorrect (doubled?) on the "mountain road" image at a particular viewport width.

justified

I could reproduce the issue in both Chrome and Firefox, Windows 10.

@Toutouwai Toutouwai changed the title Border incorrect at narrow viewport widths Margin incorrect at narrow viewport widths May 3, 2018
@acwolff
Copy link

acwolff commented May 3, 2018

It is a pity that I can’t see what the scrollbars are doing, because the problem is related to the showed/disappeared scrollbar(s).
I think too you should tell us what the values are of the refreshSensitivity, the border and the margins.
A link to your album should also be helpfull.

@Toutouwai
Copy link
Author

@acwolff, as I said above, the issue is observable in the Justified Gallery demo: http://miromannino.github.io/Justified-Gallery/

@ghost
Copy link

ghost commented May 21, 2018

I've also had the same problem for a long time and tired to hide it with a small margin, so it wouldn't be that noticeable. It would be wonderful if this bug could be fixed, as it's quite annoying.

@fireform
Copy link

fireform commented Jul 2, 2018

+1

@Brodiero
Copy link

+1

@dandan
Copy link

dandan commented Feb 27, 2019

PR #286 fixed this for me too.

cgunther added a commit to cgunther/Justified-Gallery that referenced this issue Apr 12, 2023
PR miromannino#286 attempted to fix a doubled margin between rows, however in my
use case, while it solved that, it increased the images per row, thereby
reducing the height of each image. Especially with a number of very
wide, but short, images, this led each row to be significantly shorter
then before miromannino#286.

I believe the source of that problem is that after miromannino#286, we'd buffer the
next entry, add it's aspect ratio in the `buildingRow`, then determine
if the `buildingRow` (while accounting for the aspect ratio a second
time) was below the `rowHeight` to flush the row, thereby treating
`rowHeight` like a maximum height.

It seems the intention prior to miromannino#286 was to tentatively add the next
entry's aspect ratio, without buffering it in `buildingRow` yet, to
determine if that's push us below the `rowHeight`, and if so, flush the
row before that next entry, thereby treating the `rowHeight` like a
minimum height.

In other words, before miromannino#286, we'd flush the row BEFORE adding the entry
that would push us below the configured `rowHeight`, but after miromannino#286,
we'd flush the row AFTER adding the entry that pushed us below the
`rowHeight`.

The root source of the doubled margin was flushing a row with no
buffered entries (hence increasing the offset without actually rendering
a row). Given an empty buffered entries (start of a new row), if the
entry being analyzed had an aspect ratio that'd make it's height less
than the configured `rowHeight`, we'd flush the row BEFORE buffering the
entry, thereby flushing an empty row.

Now, we only attempt to flush the row if we have at least one buffered
entry.

This should still fix miromannino#223 and miromannino#275 without introducing the side-effects
described above.
cgunther added a commit to cgunther/Justified-Gallery that referenced this issue Apr 12, 2023
PR miromannino#286 attempted to fix a doubled margin between rows, however in my
use case, while it solved that, it increased the images per row, thereby
reducing the height of each image. Especially with a number of very
wide, but short, images, this led each row to be significantly shorter
then before miromannino#286.

I believe the source of that problem is that after miromannino#286, we'd buffer the
next entry, add it's aspect ratio in the `buildingRow`, then determine
if the `buildingRow` (while accounting for the aspect ratio a second
time) was below the `rowHeight` to flush the row, thereby treating
`rowHeight` like a maximum height.

It seems the intention prior to miromannino#286 was to tentatively add the next
entry's aspect ratio, without buffering it in `buildingRow` yet, to
determine if that'd push us below the `rowHeight`, and if so, flush the
row before that next entry, thereby treating the `rowHeight` like a
minimum height.

In other words, before miromannino#286, we'd flush the row BEFORE adding the entry
that would push us below the configured `rowHeight`, but after miromannino#286,
we'd flush the row AFTER adding the entry that pushed us below the
`rowHeight`.

The root source of the doubled margin was flushing a row with no
buffered entries (hence increasing the offset without actually rendering
a row). Given an empty buffered entries (start of a new row), if the
entry being analyzed had an aspect ratio that'd make it's height less
than the configured `rowHeight`, we'd flush the row BEFORE buffering the
entry, thereby flushing an empty row.

Now, we only attempt to flush the row if we have at least one buffered
entry.

This should still fix miromannino#223 and miromannino#275 without introducing the side-effects
described above.
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 a pull request may close this issue.

5 participants