From 51d20c5920ec6cd5aa9bbe8504fd2aa59c180545 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Wed, 26 Aug 2026 08:53:52 +0200 Subject: [PATCH] fix(gallery): show other guests' colour labels in the grid (#1178) (#1180) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- backend/src/routes/gallery.js | 44 +++++++++ .../components/gallery/ColorLabelBadge.tsx | 91 +++++++++++++++---- frontend/src/components/gallery/PhotoCard.tsx | 5 +- .../__tests__/ColorLabelBadge.test.tsx | 56 ++++++++++++ .../gallery/layouts/GalleryPremiumLayout.tsx | 5 +- .../gallery/layouts/story/StoryPhotoCard.tsx | 5 +- frontend/src/i18n/locales/de.json | 3 +- frontend/src/i18n/locales/en.json | 3 +- frontend/src/i18n/locales/es.json | 3 +- frontend/src/i18n/locales/fr.json | 3 +- frontend/src/i18n/locales/nl.json | 3 +- frontend/src/i18n/locales/pt.json | 3 +- frontend/src/i18n/locales/ru.json | 3 +- frontend/src/i18n/locales/sl.json | 3 +- frontend/src/types/index.ts | 6 ++ 15 files changed, 205 insertions(+), 31 deletions(-) create mode 100644 frontend/src/components/gallery/__tests__/ColorLabelBadge.test.tsx diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index ce172f4a..82e3b3d0 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -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 diff --git a/frontend/src/components/gallery/ColorLabelBadge.tsx b/frontend/src/components/gallery/ColorLabelBadge.tsx index c8b3df86..9f89e87a 100644 --- a/frontend/src/components/gallery/ColorLabelBadge.tsx +++ b/frontend/src/components/gallery/ColorLabelBadge.tsx @@ -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 = ({ colorLabel, className = '' }) => { +export const ColorLabelBadge: React.FC = ({ + 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 ( <> -