fix(gallery): show other guests' colour labels in the grid (#1178) (#1180)

* fix(gallery): show other guests' colour labels in the grid (#1178)

A colour set by one guest was visible to others in the lightbox and invisible
on the tile. The lightbox reads /photos/:id/feedback, which returns per-colour
tallies across everyone; the grid reads /photos, whose payload carried only
`my_color_label` — so PhotoCard could render nothing else. The feature simply
was not extended to the grid.

/photos now also returns `other_color_labels`: the DISTINCT colours other
viewers put on each photo, gated on show_feedback_to_guests like every other
aggregate. `my_color_label` stays ungated, because a viewer's own selection is
not shared data — that distinction is unchanged.

Distinct colours rather than counts, and capped at three dots: a tile has room
for a couple of marks, and "who marked this, and how many" is a question the
lightbox already answers properly. The viewer's own colour is excluded from
the dots so the badge and the dots never say the same thing twice, and they
sit in opposite corners so they do not read as one group. The inset ring stays
the viewer's own signal, which is what the badge was built for.

Not addressed: the same issue asks for an identity-less shared colour tag —
one tag per photo that any guest can overwrite. Neither existing identity mode
does that (`simple` scopes by device fingerprint, `guest` by guest_id), so it
is a third model touching the feedback schema, the per-guest caps, moderation
and the admin aggregates. That is a feature with its own design, not part of
this fix.

* fix(gallery): carry other guests' labels into the premium and story grids too (#1178)

PhotoCard was not the only place the badge renders. GalleryPremiumLayout and
StoryPhotoCard have their own copies, and both still passed only
my_color_label — so the fix would have covered the default grid and left the
two full-bleed layouts showing nothing, which is the same shape of gap the
original bug had.

Found by driving a real gallery rather than reading the diff: the masonry grid
rendered the dots correctly, and a grep for the remaining call sites turned up
these two.

* fix(gallery): keep the other-viewers colour dots out of the contested corner (#1178)

The dots were placed bottom-left, which is the busiest corner in every
layout: Timeline paints a timestamp chip there on every tile, and Grid,
Mosaic and Masonry a media-type badge. All of them render after the badge,
so the dots sat underneath them.

Moved into a single row in the corner the colour-label dot already owns,
next to the viewer's own mark. Nothing new is contested, and the grouping
reads better anyway — your mark and everyone else's are the same kind of
information.

---------

Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
Paul Nothaft
2026-08-26 08:53:52 +02:00
committed by GitHub
co-authored by Paul Nothaft
parent 849a5807b7
commit 51d20c5920
15 changed files with 205 additions and 31 deletions
+44
View File
@@ -969,6 +969,46 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res)
});
}
// OTHER viewers' colour labels, per photo (#1178).
//
// The lightbox has always shown these — /photos/:id/feedback returns
// per-colour tallies across everyone — but the grid had no field carrying
// them, so a label set by one guest was visible in fullscreen and invisible
// on the tile. With sharing on, that is just a hole.
//
// DISTINCT colours, not counts: a tile has room for a couple of dots, and
// "who else marked this, and how" is a lightbox question. The viewer's own
// colour is excluded here so the badge and the dots never say the same
// thing twice — the frontend renders `my_color_label` as the badge and
// these beside it.
//
// Gated on showFeedbackToGuests, like every other aggregate: this is other
// people's feedback, unlike my_color_label above.
const otherColorLabelsByPhoto = {};
if (photos.length > 0 && showFeedbackToGuests) {
const othersQuery = db('photo_feedback')
.where({ event_id: req.event.id, feedback_type: 'color_label', is_hidden: false })
.whereIn('photo_id', photos.map(p => p.id))
.whereNotNull('color_label');
if (req.guest?.id) {
othersQuery.where(function () {
this.whereNot('guest_id', req.guest.id).orWhereNull('guest_id');
});
} else {
const mine = generateGuestIdentifier(req);
othersQuery.where(function () {
this.whereNot('guest_identifier', mine).orWhereNull('guest_identifier');
});
}
const otherRows = await othersQuery.distinct('photo_id', 'color_label');
otherRows.forEach(row => {
if (!otherColorLabelsByPhoto[row.photo_id]) otherColorLabelsByPhoto[row.photo_id] = [];
if (!otherColorLabelsByPhoto[row.photo_id].includes(row.color_label)) {
otherColorLabelsByPhoto[row.photo_id].push(row.color_label);
}
});
}
// People in each photo (#1074). Two independent gates: the feature must
// be on for this event AND, for a plain guest, the photographer must have
// left the strip visible. A client (PIN access) is the photographer's own
@@ -1249,6 +1289,10 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res)
// grid badge disappears on refresh for the very guest who set it.
color_label_count: showFeedbackToGuests ? (photo.color_label_count || 0) : 0,
my_color_label: myColorLabelByPhoto[photo.id] || null,
// Distinct colours other viewers put on this photo (#1178), so the
// grid can show them beside the viewer's own badge. Empty with
// sharing off — it is other people's feedback.
other_color_labels: otherColorLabelsByPhoto[photo.id] || [],
// People in this photo (#1074). Empty array when the feature is
// off for this event or hidden from guests, so the frontend has
// one shape to handle. Riding along on this payload is what keeps
@@ -4,6 +4,13 @@ import { COLOR_LABEL_SWATCHES, type ColorLabel } from '../../services/feedback.s
interface ColorLabelBadgeProps {
colorLabel?: string | null;
/**
* Distinct colours OTHER viewers gave this photo (#1178). Rendered as small
* dots beside the viewer's own badge, so a label set by someone else is
* visible on the tile instead of only in the lightbox. Empty when the
* gallery has feedback sharing switched off.
*/
otherColorLabels?: string[];
/** Extra classes for positioning inside the tile. */
className?: string;
}
@@ -16,31 +23,75 @@ interface ColorLabelBadgeProps {
* loud: an inset ring around the tile plus a corner dot. Both are
* pointer-events-none so they never swallow a click meant for the tile.
*/
export const ColorLabelBadge: React.FC<ColorLabelBadgeProps> = ({ colorLabel, className = '' }) => {
export const ColorLabelBadge: React.FC<ColorLabelBadgeProps> = ({
colorLabel,
otherColorLabels = [],
className = '',
}) => {
const { t } = useTranslation();
if (!colorLabel || !(colorLabel in COLOR_LABEL_SWATCHES)) return null;
const swatch = COLOR_LABEL_SWATCHES[colorLabel as ColorLabel];
const name = t(`feedback.colorLabels.${colorLabel}`, colorLabel);
const mine = colorLabel && colorLabel in COLOR_LABEL_SWATCHES ? (colorLabel as ColorLabel) : null;
// Capped at three: a tile has room for a few dots, and "exactly who marked
// this, and how many" is a question the lightbox answers properly.
const others = otherColorLabels
.filter((c) => c in COLOR_LABEL_SWATCHES && c !== colorLabel)
.slice(0, 3) as ColorLabel[];
if (!mine && others.length === 0) return null;
const swatch = mine ? COLOR_LABEL_SWATCHES[mine] : null;
const name = mine ? t(`feedback.colorLabels.${mine}`, mine) : '';
const othersLabel = t('feedback.alsoMarkedBy', 'Also marked by others: {{colors}}', {
colors: others.map((c) => t(`feedback.colorLabels.${c}`, c)).join(', '),
});
return (
<>
<span
className={`absolute inset-0 pointer-events-none rounded-[inherit] ${className}`}
// Inset rather than an outline: the tile is often flush against its
// neighbours in masonry/justified layouts, where an outer ring would
// be clipped.
style={{ boxShadow: `inset 0 0 0 3px ${swatch.fill}` }}
aria-hidden="true"
/>
<span
className="absolute top-2 left-2 pointer-events-none flex items-center justify-center w-5 h-5 rounded-full border-2 border-white/90 shadow"
style={{ backgroundColor: swatch.fill }}
// Colour alone can't carry the meaning — the accessible name does.
title={t('feedback.markedAs', 'Marked as {{color}}', { color: name })}
role="img"
aria-label={t('feedback.markedAs', 'Marked as {{color}}', { color: name })}
/>
{mine && swatch && (
<span
className={`absolute inset-0 pointer-events-none rounded-[inherit] ${className}`}
// Inset rather than an outline: the tile is often flush against its
// neighbours in masonry/justified layouts, where an outer ring would
// be clipped.
style={{ boxShadow: `inset 0 0 0 3px ${swatch.fill}` }}
aria-hidden="true"
/>
)}
{/* One row in the corner the colour-label dot already owns, rather than a
second corner of its own. Bottom-left is taken across the layouts —
Timeline puts a timestamp chip there on every tile, Grid/Mosaic/
Masonry a media-type badge — and anything placed there gets painted
over. Sharing this position also reads better: your mark and everyone
else's are the same kind of information. */}
<span className="absolute top-2 left-2 pointer-events-none flex items-center gap-1">
{mine && swatch && (
<span
className="flex items-center justify-center w-5 h-5 rounded-full border-2 border-white/90 shadow"
style={{ backgroundColor: swatch.fill }}
// Colour alone can't carry the meaning — the accessible name does.
title={t('feedback.markedAs', 'Marked as {{color}}', { color: name })}
role="img"
aria-label={t('feedback.markedAs', 'Marked as {{color}}', { color: name })}
/>
)}
{others.length > 0 && (
<span
className="flex items-center gap-1"
role="img"
aria-label={othersLabel}
title={othersLabel}
>
{others.map((c) => (
<span
key={c}
className="block w-2.5 h-2.5 rounded-full border border-white/90 shadow-sm"
style={{ backgroundColor: COLOR_LABEL_SWATCHES[c].fill }}
/>
))}
</span>
)}
</span>
</>
);
};
@@ -382,7 +382,10 @@ export const PhotoCard: React.FC<PhotoCardProps> = ({
{/* The guest's own colour label (#1044) — visible without hovering
or opening anything, which is the point: the client watches
their selection progress across the grid. */}
<ColorLabelBadge colorLabel={photo.my_color_label} />
<ColorLabelBadge
colorLabel={photo.my_color_label}
otherColorLabels={photo.other_color_labels}
/>
{beforeOverlay}
@@ -0,0 +1,56 @@
/**
* Other guests' colour labels must reach the grid (#1178).
*
* The lightbox has always shown them — /photos/:id/feedback returns per-colour
* tallies across everyone — but the grid payload carried only the viewer's own
* label, so a colour set by one guest was visible in fullscreen and invisible
* on the tile. With "Show Feedback to Guests" on, that is just a hole.
*/
import { describe, it, expect } from 'vitest';
import { render, screen } from '@testing-library/react';
import { ColorLabelBadge } from '../ColorLabelBadge';
describe('ColorLabelBadge (#1178)', () => {
it('renders nothing when there is no label at all', () => {
const { container } = render(<ColorLabelBadge colorLabel={null} />);
expect(container).toBeEmptyDOMElement();
});
it("shows only the viewer's own badge when nobody else marked it", () => {
render(<ColorLabelBadge colorLabel="red" />);
expect(screen.getByLabelText(/Marked as/i)).toBeInTheDocument();
expect(screen.queryByLabelText(/Also marked by others/i)).not.toBeInTheDocument();
});
it("shows other guests' colours even when the viewer has none", () => {
// The reported case: another guest set red, this viewer set nothing, and
// the tile showed no indication at all.
render(<ColorLabelBadge colorLabel={null} otherColorLabels={['red']} />);
expect(screen.getByLabelText(/Also marked by others/i)).toBeInTheDocument();
});
it('shows both without repeating the viewers own colour', () => {
// Own colour excluded from the dots so the badge and the dots never say
// the same thing twice. Asserted on the rendered dots rather than the
// aria-label, because the test i18n returns the raw default string without
// interpolating {{colors}}.
render(<ColorLabelBadge colorLabel="red" otherColorLabels={['red', 'green']} />);
expect(screen.getByLabelText(/Marked as/i)).toBeInTheDocument();
const dots = screen.getByLabelText(/Also marked by others/i).querySelectorAll('span');
expect(dots.length).toBe(1);
});
it('caps the dots so a heavily-marked photo cannot flood the tile', () => {
render(<ColorLabelBadge colorLabel={null} otherColorLabels={['red', 'green', 'blue', 'yellow', 'purple']} />);
const others = screen.getByLabelText(/Also marked by others/i);
expect(others.querySelectorAll('span').length).toBe(3);
});
it('ignores a colour it does not know', () => {
// Forward-compat: a colour added server-side that this build has no swatch
// for must not render a blank dot.
render(<ColorLabelBadge colorLabel={null} otherColorLabels={['chartreuse']} />);
expect(screen.queryByLabelText(/Also marked by others/i)).not.toBeInTheDocument();
});
});
@@ -133,7 +133,10 @@ const PhotoCard: React.FC<PhotoCardProps> = ({
/>
{/* Colour label (#1044) — same badge every layout uses. */}
<ColorLabelBadge colorLabel={photo.my_color_label} />
<ColorLabelBadge
colorLabel={photo.my_color_label}
otherColorLabels={photo.other_color_labels}
/>
{/* Overlay Gradient */}
<div className="gallery-premium-photo-overlay" />
@@ -117,7 +117,10 @@ export const StoryPhotoCard: React.FC<StoryPhotoCardProps> = ({
</a>
{/* Colour label (#1044) — same badge every layout uses. */}
<ColorLabelBadge colorLabel={photo.my_color_label} />
<ColorLabelBadge
colorLabel={photo.my_color_label}
otherColorLabels={photo.other_color_labels}
/>
{/* Overlay */}
<div className="story-photo-card-overlay" />
+2 -1
View File
@@ -3827,7 +3827,8 @@
"colorLabelError": "Farbmarkierung konnte nicht gespeichert werden",
"removeColorLabel": "Markierung {{color}} entfernen",
"setColorLabel": "Als {{color}} markieren",
"markedAs": "Markiert als {{color}}"
"markedAs": "Markiert als {{color}}",
"alsoMarkedBy": "Auch von anderen markiert: {{colors}}"
},
"filter": {
"feedbackFilters": "Feedback-Filter",
+2 -1
View File
@@ -3848,7 +3848,8 @@
"colorLabelError": "Failed to update color label",
"removeColorLabel": "Remove {{color}} label",
"setColorLabel": "Mark as {{color}}",
"markedAs": "Marked as {{color}}"
"markedAs": "Marked as {{color}}",
"alsoMarkedBy": "Also marked by others: {{colors}}"
},
"filter": {
"feedbackFilters": "Feedback Filters",
+2 -1
View File
@@ -2473,7 +2473,8 @@
"colorLabelError": "No se pudo actualizar la etiqueta de color",
"removeColorLabel": "Quitar la etiqueta {{color}}",
"setColorLabel": "Marcar como {{color}}",
"markedAs": "Marcado como {{color}}"
"markedAs": "Marcado como {{color}}",
"alsoMarkedBy": "También marcado por otros: {{colors}}"
},
"filter": {
"feedbackFilters": "Filtros de feedback",
+2 -1
View File
@@ -2701,7 +2701,8 @@
"colorLabelError": "Échec de la mise à jour de l'étiquette de couleur",
"removeColorLabel": "Retirer l'étiquette {{color}}",
"setColorLabel": "Marquer comme {{color}}",
"markedAs": "Marqué comme {{color}}"
"markedAs": "Marqué comme {{color}}",
"alsoMarkedBy": "Également marqué par dautres : {{colors}}"
},
"filter": {
"feedbackFilters": "Filtres de commentaires",
+2 -1
View File
@@ -2679,7 +2679,8 @@
"colorLabelError": "Bijwerken van kleurlabel mislukt",
"removeColorLabel": "Label {{color}} verwijderen",
"setColorLabel": "Markeren als {{color}}",
"markedAs": "Gemarkeerd als {{color}}"
"markedAs": "Gemarkeerd als {{color}}",
"alsoMarkedBy": "Ook door anderen gemarkeerd: {{colors}}"
},
"filter": {
"feedbackFilters": "Feedbackfilters",
+2 -1
View File
@@ -2709,7 +2709,8 @@
"colorLabelError": "Não foi possível atualizar a etiqueta de cor",
"removeColorLabel": "Remover a etiqueta {{color}}",
"setColorLabel": "Marcar como {{color}}",
"markedAs": "Marcado como {{color}}"
"markedAs": "Marcado como {{color}}",
"alsoMarkedBy": "Também marcado por outros: {{colors}}"
},
"filter": {
"feedbackFilters": "Filtros de Feedback",
+2 -1
View File
@@ -2739,7 +2739,8 @@
"colorLabelError": "Не удалось обновить цветовую метку",
"removeColorLabel": "Убрать метку «{{color}}»",
"setColorLabel": "Отметить как «{{color}}»",
"markedAs": "Отмечено как «{{color}}»"
"markedAs": "Отмечено как «{{color}}»",
"alsoMarkedBy": "Также отмечено другими: {{colors}}"
},
"filter": {
"feedbackFilters": "Фильтры отзывов",
+2 -1
View File
@@ -2690,7 +2690,8 @@
"colorLabelError": "Barvne oznake ni bilo mogoče posodobiti",
"removeColorLabel": "Odstrani oznako {{color}}",
"setColorLabel": "Označi kot {{color}}",
"markedAs": "Označeno kot {{color}}"
"markedAs": "Označeno kot {{color}}",
"alsoMarkedBy": "Označili tudi drugi: {{colors}}"
},
"filter": {
"feedbackFilters": "Filtri povratnih informacij",
+6
View File
@@ -217,6 +217,12 @@ export interface Photo {
// galleries where feedback isn't shared between guests.
color_label_count?: number;
my_color_label?: string | null;
// Distinct colours OTHER viewers gave this photo (#1178). The lightbox has
// always shown these as per-colour tallies; without this field the grid
// could only ever render the viewer's own label, so a colour set by someone
// else was visible in fullscreen and invisible on the tile. Empty when the
// gallery has show_feedback_to_guests off — it is other people's feedback.
other_color_labels?: string[];
}
// Download resolutions (#858).