refactor(gallery): let the parent layout own all inter-card spacing
JustifiedGridCell.BORDER_BLEED shrank each card's image away from its own edges on every side, and since JustifiedGalleryPane.SPACING was 0, that per-card inset was actually the only thing producing any visible gap between thumbnails — spacing was split between the parent layout (nominally) and a card-internal padding (in practice). SPACING is now 6 and the sole source of gap, both across a row (HBox spacing) and between rows (ListCell top padding) — a card has no padding/inset of its own any more, its ImageView fills the card's bounds edge to edge. The navigation-selection frame follows: it now paints directly over the ImageView's outer edge (SELECTION_FRAME_WIDTH, renamed from BORDER_BLEED, is a purely visual stroke width with no tie to layout spacing any more) rather than sitting in a margin that no longer exists. Also fixed a stale resizeCard javadoc left over from the width- stretching mechanism removed in an earlier commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012sfcVC7rYy5k8XiKCbh4Tr
This commit is contained in:
@@ -105,13 +105,12 @@ public class JustifiedGalleryPane extends StackPane implements Disposable {
|
||||
|
||||
/**
|
||||
* Gap between adjacent same-day thumbnail <em>cards</em>, in pixels — both across a row
|
||||
* ({@link JustifiedGridCell}'s {@code photosBox}) and between rows ({@link JustifiedRow#justify}'s
|
||||
* row-width packing). Zero: card bounds touch directly. The visible gap between two <em>images</em> is
|
||||
* never actually zero even so — each card already reserves {@link JustifiedGridCell#BORDER_BLEED} of its
|
||||
* own on every side for its selection/hover border ring, so two adjacent images end up exactly
|
||||
* {@code 2 * BORDER_BLEED} apart regardless of this value.
|
||||
* ({@link JustifiedGridCell}'s {@code photosBox}) and between rows (this pane's own {@code ListCell}
|
||||
* top padding, set from this same constant). The only source of visible space between two cards: a card
|
||||
* has no padding or inset of its own any more — its {@code ImageView} fills its bounds edge to edge —
|
||||
* so this layout-level gap is authoritative, not a floor under some other margin.
|
||||
*/
|
||||
static final double SPACING = 0;
|
||||
static final double SPACING = 6;
|
||||
|
||||
/**
|
||||
* Extra gap, beyond {@link #SPACING}, before the one entry that starts a new date when that date's
|
||||
|
||||
@@ -58,9 +58,9 @@ import java.util.function.*;
|
||||
* that can lag behind the flip itself on a virtualised cell under a heavy stylesheet. {@link #selectionFrame}
|
||||
* sidesteps that: {@link #notifyNavigationSelected}/{@link #notifyNavigationDeselected} — {@code
|
||||
* JustifiedGalleryPane.select}'s own hand-off, see that method's javadoc — just call {@link Node#setVisible}
|
||||
* on it directly. No rounding, no drop-shadow: a plain rectangular ring, painted entirely inside this card's
|
||||
* own bounds, in the same {@link #BORDER_BLEED} margin the thumbnail itself is already inset by — see
|
||||
* {@link #selectionFrame}'s own javadoc for the exact sizing.
|
||||
* on it directly. No rounding, no drop-shadow: a plain rectangular ring, painted directly over this card's own
|
||||
* {@code ImageView} — a card has no padding or inset of its own for it to sit in instead, see
|
||||
* {@link #selectionFrame}'s own javadoc.
|
||||
*
|
||||
* <p>The grid's other, independent selection — {@code JustifiedGalleryPane}'s own {@code actionSelection},
|
||||
* 0..N files for a future group action — has no visual yet; a cell will <em>pull</em>
|
||||
@@ -727,10 +727,9 @@ final class JustifiedGridCell extends ListCell<JustifiedGridItem> {
|
||||
}
|
||||
|
||||
/**
|
||||
* A fixed-size wrapper, exactly {@code width}/{@code height} shrunk on every side by {@link #BORDER_BLEED}
|
||||
* — never the raw {@link ImageView} directly — so {@code card} (the {@link StackPane} this is added
|
||||
* into) centres it with that same margin all round, left empty for {@link #selectionFrame}'s own ring.
|
||||
* The wrapper's own clip, not one on {@code card} itself, is what crops the image.
|
||||
* A fixed-size wrapper, exactly {@code width}×{@code height} — the card's own full bounds, no inset —
|
||||
* never the raw {@link ImageView} directly, so {@code card} (the {@link StackPane} this is added into)
|
||||
* has something whose own clip, not one on {@code card} itself, crops the image.
|
||||
*
|
||||
* <p>{@code height} is always exact, never cropped: the card's height is the one dimension every row
|
||||
* shares. Width instead "covers": scaled at least as wide as the wrapper even if the decoded image's
|
||||
@@ -739,8 +738,8 @@ final class JustifiedGridCell extends ListCell<JustifiedGridItem> {
|
||||
* reused card's width changes across renders.
|
||||
*/
|
||||
private static StackPane imageView(Image image, double width, double height) {
|
||||
double fullTargetHeight = Math.max(1, height - 2 * BORDER_BLEED);
|
||||
double fullTargetWidth = Math.max(1, width - 2 * BORDER_BLEED);
|
||||
double fullTargetHeight = Math.max(1, height);
|
||||
double fullTargetWidth = Math.max(1, width);
|
||||
double naturalWidth = image.getWidth() * fullTargetHeight / image.getHeight();
|
||||
|
||||
ImageView view = new ImageView(image);
|
||||
@@ -764,18 +763,16 @@ final class JustifiedGridCell extends ListCell<JustifiedGridItem> {
|
||||
}
|
||||
|
||||
/**
|
||||
* Unlike {@code legacy.GalleryGridCell} (a per-file width there is a pure function of height alone, so
|
||||
* a reused card's width can never actually change across renders that keep the same height/quality), a
|
||||
* {@link JustifiedRow} entry's width also depends on the rest of its row (see {@link JustifiedRow#justify}'s
|
||||
* stretching step) — so even a render that reuses this exact card for the exact same file can still need
|
||||
* a different width than last time, whenever a resize shifted how much slack this row's stretch had to
|
||||
* distribute. This is layout-only (no re-decode): the wrapper's own size/clip, and — since the
|
||||
* {@link ImageView}'s {@code fitWidth} is "cover"-computed from the image's own natural aspect, see
|
||||
* {@link #imageView} — the {@link ImageView}'s own {@code fitWidth} too, so a widened card never leaves a
|
||||
* gap of unfilled background down one side. Called unconditionally on every reused card; harmless (a
|
||||
* same-size re-application) whenever the width in fact didn't change. Never touches
|
||||
* {@link #selectionFrame} — its width/height are bound to {@code card}'s own, and it carries no rounding
|
||||
* tied to any state, so there is nothing here for it to fall out of sync with.
|
||||
* A resize/re-chunk can hand this exact same card, for the exact same file, a different width than last
|
||||
* time — {@link JustifiedRow.Entry#width()} is fixed at that row's own natural width, but a viewport
|
||||
* resize can still re-chunk which files share a row, or how many entries land in this one, changing its
|
||||
* width even though the file itself didn't change. This is layout-only (no re-decode): the wrapper's own
|
||||
* size/clip, and — since the {@link ImageView}'s {@code fitWidth} is "cover"-computed from the image's
|
||||
* own natural aspect, see {@link #imageView} — the {@link ImageView}'s own {@code fitWidth} too, so a
|
||||
* widened card never leaves a gap of unfilled background down one side. Called unconditionally on every
|
||||
* reused card; harmless (a same-size re-application) whenever the width in fact didn't change. Never
|
||||
* touches {@link #selectionFrame} — its width/height are bound to {@code card}'s own, and it carries no
|
||||
* rounding tied to any state, so there is nothing here for it to fall out of sync with.
|
||||
*/
|
||||
private static void resizeCard(StackPane card, double width, double height) {
|
||||
card.setMinSize(width, height);
|
||||
@@ -784,8 +781,8 @@ final class JustifiedGridCell extends ListCell<JustifiedGridItem> {
|
||||
if (card.getChildren().isEmpty() || !(card.getChildren().getFirst() instanceof StackPane content)) {
|
||||
return;
|
||||
}
|
||||
double targetWidth = Math.max(1, width - 2 * BORDER_BLEED);
|
||||
double targetHeight = Math.max(1, height - 2 * BORDER_BLEED);
|
||||
double targetWidth = Math.max(1, width);
|
||||
double targetHeight = Math.max(1, height);
|
||||
fixedSize(content, targetWidth, targetHeight);
|
||||
if (!content.getChildren().isEmpty() && content.getChildren().getFirst() instanceof ImageView imageView) {
|
||||
Image image = imageView.getImage();
|
||||
@@ -828,29 +825,20 @@ final class JustifiedGridCell extends ListCell<JustifiedGridItem> {
|
||||
}
|
||||
|
||||
/**
|
||||
* Reserved ring, in pixels, every card leaves empty around its own thumbnail on every side (via
|
||||
* {@link #fixedSize}) — this is what {@link #selectionFrame} paints its own stroke into, and exactly
|
||||
* how thick that stroke is (see that method's own javadoc for why they must match exactly). Bumped from
|
||||
* this component's original {@code 2} to {@code 3}: the earlier value was only ever a card's own share
|
||||
* of the gap between two side-by-side thumbnails ({@code 2 * BORDER_BLEED}, since
|
||||
* {@code JustifiedGalleryPane.SPACING} is {@code 0} — cards touch directly, this ring is the only thing
|
||||
* separating them) — {@code 3} keeps the selection frame visibly thicker than that per-card share was,
|
||||
* without needing to touch {@code SPACING} or grow the gap between UNselected neighbours any further
|
||||
* than this one extra pixel already does.
|
||||
*
|
||||
* <p>Package-private: {@code JustifiedGalleryPane.SPACING} being {@code 0} is exactly why this ring
|
||||
* matters — it is the only thing keeping two adjacent images from touching directly.
|
||||
* {@link #selectionFrame}'s own stroke width, in pixels — a purely visual choice, independent of any
|
||||
* layout spacing: a card has no padding/inset of its own any more (see this class's own javadoc), so the
|
||||
* frame paints directly over the outer edge of the {@code ImageView} rather than into a reserved margin.
|
||||
*/
|
||||
static final double BORDER_BLEED = 3;
|
||||
private static final double SELECTION_FRAME_WIDTH = 3;
|
||||
|
||||
/**
|
||||
* The Google-Photos-style navigation-selection frame: a plain rectangular ring, no rounding, no
|
||||
* drop-shadow — {@code accentColor}, resolved once at construction (see that field's own javadoc), for
|
||||
* the stroke; {@link javafx.scene.paint.Color#TRANSPARENT} fill so it never obscures the photo.
|
||||
* {@link StrokeType#INSIDE} at exactly {@link #BORDER_BLEED} wide is what keeps the whole ring painted
|
||||
* strictly within {@code card}'s own bounds — inside the same margin {@link #imageView}/
|
||||
* {@link #pendingThumbnail} already leave the thumbnail short of on every side, touching its outer edge
|
||||
* without ever overlapping it, and never spilling into a neighbouring card's own territory (impossible
|
||||
* the stroke; {@link javafx.scene.paint.Color#TRANSPARENT} fill so it never obscures the rest of the
|
||||
* photo. {@link StrokeType#INSIDE} at exactly {@link #SELECTION_FRAME_WIDTH} wide keeps the whole ring
|
||||
* painted strictly within {@code card}'s own bounds — deliberately overlapping the outer few pixels of
|
||||
* the {@code ImageView} itself, which fills those same bounds edge to edge with no margin left for the
|
||||
* frame to sit in instead — and never spilling into a neighbouring card's own territory (impossible
|
||||
* regardless, since {@code card}'s bounds are what this is clipped to, but by design as much as by
|
||||
* construction). Bound to {@code card}'s own live {@code width}/{@code height}, not a fixed snapshot, so
|
||||
* {@link #resizeCard} moving a reused card to a new width needs no separate step to keep it in sync.
|
||||
@@ -868,7 +856,7 @@ final class JustifiedGridCell extends ListCell<JustifiedGridItem> {
|
||||
frame.heightProperty().bind(card.heightProperty());
|
||||
frame.setFill(Color.TRANSPARENT);
|
||||
frame.setStroke(accentColor.get());
|
||||
frame.setStrokeWidth(BORDER_BLEED);
|
||||
frame.setStrokeWidth(SELECTION_FRAME_WIDTH);
|
||||
frame.setStrokeType(StrokeType.INSIDE);
|
||||
frame.setMouseTransparent(true);
|
||||
return frame;
|
||||
@@ -925,6 +913,6 @@ final class JustifiedGridCell extends ListCell<JustifiedGridItem> {
|
||||
icon.getStyleClass().add("thumbnail-placeholder-icon");
|
||||
StackPane box = new StackPane(icon);
|
||||
box.getStyleClass().add("thumbnail-placeholder");
|
||||
return fixedSize(box, Math.max(1, width - 2 * BORDER_BLEED), Math.max(1, height - 2 * BORDER_BLEED));
|
||||
return fixedSize(box, Math.max(1, width), Math.max(1, height));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user