Skip to content

Commit

Permalink
Properly fix wrong offset calculation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cgunther committed Apr 11, 2023
1 parent c8ef4d2 commit 877d326
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 25 deletions.
2 changes: 1 addition & 1 deletion dist/css/justifiedGallery.css
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*!
* justifiedGallery - v3.8.1
* http://miromannino.github.io/Justified-Gallery/
* Copyright (c) 2020 Miro Mannino
* Copyright (c) 2023 Miro Mannino
* Licensed under the MIT license.
*/
.justified-gallery {
Expand Down
2 changes: 1 addition & 1 deletion dist/css/justifiedGallery.min.css
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*!
* justifiedGallery - v3.8.1
* http://miromannino.github.io/Justified-Gallery/
* Copyright (c) 2020 Miro Mannino
* Copyright (c) 2023 Miro Mannino
* Licensed under the MIT license.
*/
.justified-gallery {
Expand Down
22 changes: 11 additions & 11 deletions dist/js/jquery.justifiedGallery.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*!
* justifiedGallery - v3.8.1
* http://miromannino.github.io/Justified-Gallery/
* Copyright (c) 2020 Miro Mannino
* Copyright (c) 2023 Miro Mannino
* Licensed under the MIT license.
*/
(function (factory) {
Expand Down Expand Up @@ -223,7 +223,7 @@
});

var loadNewImage = function () {
// if (imageSrc !== newImageSrc) {
// if (imageSrc !== newImageSrc) {
$image.attr('src', imageSrc);
// }
};
Expand All @@ -236,7 +236,7 @@
} else {
this.showImg($entry, loadNewImage); //load the new image after the fadeIn
}

}

} else {
Expand Down Expand Up @@ -795,19 +795,19 @@
(this.buildingRow.entriesBuff.length - 1) * this.settings.margins);
var imgAspectRatio = $entry.data('jg.width') / $entry.data('jg.height');

this.buildingRow.entriesBuff.push($entry);
this.buildingRow.aspectRatio += imgAspectRatio;
this.buildingRow.width += imgAspectRatio * this.settings.rowHeight;
this.lastAnalyzedIndex = i;

if (availableWidth / (this.buildingRow.aspectRatio + imgAspectRatio) < this.settings.rowHeight) {
if (this.buildingRow.entriesBuff.length > 0 && availableWidth / (this.buildingRow.aspectRatio + imgAspectRatio) < this.settings.rowHeight) {
this.flushRow(false, this.settings.maxRowsCount > 0 && this.rows === this.settings.maxRowsCount);

if (++this.yield.flushed >= this.yield.every) {
this.startImgAnalyzer(isForResize);
return;
}
}

this.buildingRow.entriesBuff.push($entry);
this.buildingRow.aspectRatio += imgAspectRatio;
this.buildingRow.width += imgAspectRatio * this.settings.rowHeight;
this.lastAnalyzedIndex = i;
} else if ($entry.data('jg.loaded') !== 'error') {
return;
}
Expand All @@ -829,7 +829,7 @@
this.stopImgAnalyzerStarter();

this.setGalleryFinalHeight(this.galleryHeightToSet);

//On complete callback
this.settings.triggerEvent.call(this, isForResize ? 'jg.resize' : 'jg.complete');
};
Expand Down Expand Up @@ -911,7 +911,7 @@
// Image src
var imageSrc = that.extractImgSrcFromImage($image);

/* If we have the height and the width, we don't wait that the image is loaded,
/* If we have the height and the width, we don't wait that the image is loaded,
but we start directly with the justification */
if (that.settings.waitThumbnailsLoad === false || !imageSrc) {
var width = parseFloat($image.attr('width'));
Expand Down
4 changes: 2 additions & 2 deletions dist/js/jquery.justifiedGallery.min.js

Large diffs are not rendered by default.

Binary file modified dist/justifiedGallery.min.zip
Binary file not shown.
Binary file modified dist/justifiedGallery.zip
Binary file not shown.
20 changes: 10 additions & 10 deletions src/js/justifiedGallery.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ JustifiedGallery.prototype.displayEntry = function ($entry, x, y, imgWidth, imgH
});

var loadNewImage = function () {
// if (imageSrc !== newImageSrc) {
// if (imageSrc !== newImageSrc) {
$image.attr('src', imageSrc);
// }
};
Expand All @@ -210,7 +210,7 @@ JustifiedGallery.prototype.displayEntry = function ($entry, x, y, imgWidth, imgH
} else {
this.showImg($entry, loadNewImage); //load the new image after the fadeIn
}

}

} else {
Expand Down Expand Up @@ -769,19 +769,19 @@ JustifiedGallery.prototype.analyzeImages = function (isForResize) {
(this.buildingRow.entriesBuff.length - 1) * this.settings.margins);
var imgAspectRatio = $entry.data('jg.width') / $entry.data('jg.height');

this.buildingRow.entriesBuff.push($entry);
this.buildingRow.aspectRatio += imgAspectRatio;
this.buildingRow.width += imgAspectRatio * this.settings.rowHeight;
this.lastAnalyzedIndex = i;

if (availableWidth / (this.buildingRow.aspectRatio + imgAspectRatio) < this.settings.rowHeight) {
if (this.buildingRow.entriesBuff.length > 0 && availableWidth / (this.buildingRow.aspectRatio + imgAspectRatio) < this.settings.rowHeight) {
this.flushRow(false, this.settings.maxRowsCount > 0 && this.rows === this.settings.maxRowsCount);

if (++this.yield.flushed >= this.yield.every) {
this.startImgAnalyzer(isForResize);
return;
}
}

this.buildingRow.entriesBuff.push($entry);
this.buildingRow.aspectRatio += imgAspectRatio;
this.buildingRow.width += imgAspectRatio * this.settings.rowHeight;
this.lastAnalyzedIndex = i;
} else if ($entry.data('jg.loaded') !== 'error') {
return;
}
Expand All @@ -803,7 +803,7 @@ JustifiedGallery.prototype.analyzeImages = function (isForResize) {
this.stopImgAnalyzerStarter();

this.setGalleryFinalHeight(this.galleryHeightToSet);

//On complete callback
this.settings.triggerEvent.call(this, isForResize ? 'jg.resize' : 'jg.complete');
};
Expand Down Expand Up @@ -885,7 +885,7 @@ JustifiedGallery.prototype.init = function () {
// Image src
var imageSrc = that.extractImgSrcFromImage($image);

/* If we have the height and the width, we don't wait that the image is loaded,
/* If we have the height and the width, we don't wait that the image is loaded,
but we start directly with the justification */
if (that.settings.waitThumbnailsLoad === false || !imageSrc) {
var width = parseFloat($image.attr('width'));
Expand Down

0 comments on commit 877d326

Please sign in to comment.