Skip to content

fix(compare): horizontally scrollable comparison table on mobile - #1735

Open
shashwat-28410 wants to merge 4 commits into
openfoodfacts:mainfrom
shashwat-28410:fix/compare-horizontal-scroll
Open

shashwat-28410 wants to merge 4 commits into
openfoodfacts:mainfrom
shashwat-28410:fix/compare-horizontal-scroll

Conversation

@shashwat-28410

@shashwat-28410 shashwat-28410 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Replaces the vertical card-based mobile compare view with the
existing responsive table (already used on desktop) across all
screen sizes. The table already has overflow-x-auto and asticky first column, so on mobile users can swipe left/right
to compare products side by side while nutrient labels stay pinned.

This directly addresses teolemon's feedback on #1722: "The real
plus on compare mode on mobile would be horizontal scrolling."

What changed:

  • Removed the lg:hidden/lg:block split — the table now renders
    at all screen sizes.
  • Removed the entire mobile card view (now redundant).
  • Removed the unused nutrientValue snippet (was only used by the
    card view; the table uses nutrientValueDesktop).

Net result: -130 lines, one unified responsive table.

Screenshot or video

Screenshot 2026-08-05 at 9 56 52 PM

Related issue(s) and discussion

Follow-up to the review discussion on PR #1722.

Checklist: Author Self-Review

  • I have performed a self-review of my own code (including running it).
  • I understand the changes I'm proposing and why they are needed.
  • My changes generate no new warnings or errors (linting, console).
  • I have made corresponding changes to the documentation (if applicable).

Large Language Models usage disclosure

  • I used an LLM / AI agent — details below:
    • Agent / tool name and version: Claude Code (Anthropic)
    • How it was used: agentic — explored the codebase, removed the mobile card view and dead code, updated responsive layout
  • I have reviewed and take full responsibility for all AI-generated code in this PR.

Summary by CodeRabbit

  • New Features

    • Added horizontal scrolling and snap navigation for mobile comparison cards.
    • Added controls to reorder compared products.
    • Improved product imagery, typography, and nutrient value presentation.
    • Increased desktop comparison table width for improved readability.
  • Accessibility

    • Updated the Nova Group accessibility label.

@vercel

vercel Bot commented Aug 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
explorer Ready Ready Preview Aug 9, 2026 3:26am

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f5680490-ac20-4d3c-a5d8-56abf5698d8b

📥 Commits

Reviewing files that changed from the base of the PR and between dd9cad3 and 936bf16.

📒 Files selected for processing (1)
  • src/lib/ui/ComparisonDisplay.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/ui/ComparisonDisplay.svelte

📝 Walkthrough

Walkthrough

The comparison display updates mobile cards with horizontal scrolling, snap alignment, reordering, refreshed styling, and compact nutrient values. It also changes the Nova Group accessibility label and adjusts desktop table width and product column sizing.

Changes

Comparison layout

Layer / File(s) Summary
Responsive comparison presentation
src/lib/ui/ComparisonDisplay.svelte
Mobile cards now support scrolling, snapping, product reordering, revised imagery and typography, and compact nutrient values without comparison badges or differences. The Nova Group accessibility label and desktop table sizing are updated.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: Compare mode

Poem

A rabbit hops through cards in a row,
With snap and scroll wherever they go.
Nutrients stand neat, badges take flight,
NOVA speaks clearer, tables grow wide.
Product columns line up just right—
“Compare!” cries Bunny, pleased tonight.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: making the comparison table horizontally scrollable on mobile.
Description check ✅ Passed The description explains the change, includes visual evidence, related discussion, self-review details, and AI usage disclosure.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@shashwat-28410

Copy link
Copy Markdown
Contributor Author

@teolemon check this i have added the horizontal scrolling

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/lib/ui/PwaInstallButton.svelte (1)

18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add default fallbacks to all new PWA translation calls.

The new PWA UI calls $_() without the required default option. Add the English fallback at every call site.

  • src/lib/ui/PwaInstallButton.svelte#L18-L18: add the success-message fallback.
  • src/lib/ui/PwaInstallButton.svelte#L44-L44: add the success-message fallback.
  • src/lib/ui/PwaInstallButton.svelte#L50-L50: add the error-message fallback.
  • src/lib/ui/PwaInstallButton.svelte#L61-L61: add fallbacks for both button labels.
  • src/routes/settings/+page.svelte#L230-L232: add fallbacks for the label and description.

As per coding guidelines, “All user-facing strings must go through svelte-i18n, and calls must include a default fallback.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/ui/PwaInstallButton.svelte` at line 18, Add the appropriate English
default fallback to every new PWA translation call using $_: both success
messages, the error message, and both button labels in PwaInstallButton.svelte,
plus the settings label and description in src/routes/settings/+page.svelte at
lines 230-232. Update the call sites in PwaInstallButton.svelte at lines 18, 44,
50, and 61, preserving the existing translation keys and UI behavior.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/ui/PwaInstallButton.svelte`:
- Around line 43-48: Update the installation completion handling in
PwaInstallButton around the userChoice outcome and appinstalled listener so
accepted installations do not trigger duplicate success toasts; coordinate or
deduplicate both success paths while preserving the dismissed behavior and
prompt clearing.

---

Nitpick comments:
In `@src/lib/ui/PwaInstallButton.svelte`:
- Line 18: Add the appropriate English default fallback to every new PWA
translation call using $_: both success messages, the error message, and both
button labels in PwaInstallButton.svelte, plus the settings label and
description in src/routes/settings/+page.svelte at lines 230-232. Update the
call sites in PwaInstallButton.svelte at lines 18, 44, 50, and 61, preserving
the existing translation keys and UI behavior.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e7393939-eefe-45cd-b625-472b3b71268a

📥 Commits

Reviewing files that changed from the base of the PR and between 21a57b3 and edc3e07.

📒 Files selected for processing (7)
  • src/lib/i18n/messages/en.json
  • src/lib/stores/pwa.ts
  • src/lib/ui/ComparisonDisplay.svelte
  • src/lib/ui/PwaInstallButton.svelte
  • src/routes/+layout.svelte
  • src/routes/settings/+page.svelte
  • src/routes/site.webmanifest/+server.ts

Comment thread src/lib/ui/PwaInstallButton.svelte Outdated
Comment on lines +43 to +48
if (outcome === 'accepted') {
toastCtx.success($_('pwa.install_success'));
}
// Whether accepted or dismissed, clear the stored prompt
// — the browser won't fire `beforeinstallprompt` again.
pwaInstallStore.clear();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Files matching PwaInstallButton:\n'
fd -a 'PwaInstallButton\.svelte$' . || true

printf '\nRelevant file outline:\n'
file="$(fd 'PwaInstallButton\.svelte$' . | head -n 1)"
if [ -n "${file:-}" ]; then
  wc -l "$file"
  ast-grep outline "$file" || true
  printf '\nFile contents with line numbers:\n'
  cat -n "$file"
fi

printf '\nSearch for pwaInstallStore / handleAppInstalled / install_success references:\n'
rg -n "pwaInstallStore|handleAppInstalled|install_success|appinstalled|userChoice|beforeinstallprompt" .

Repository: openfoodfacts/openfoodfacts-explorer

Length of output: 4605


🌐 Web query:

MDN BeforeInstallPromptEvent appinstalled event userChoice accepted developer.mozilla.org

💡 Result:

The BeforeInstallPromptEvent, userChoice, and appinstalled event are related components of the Progressive Web App (PWA) installation process, primarily supported in Chromium-based browsers [1][2]. BeforeInstallPromptEvent and userChoice When a browser detects that a web app is installable, it fires a beforeinstallprompt event [3]. If a developer catches this event (and calls event.preventDefault() to suppress the default browser prompt), they can save the event object and trigger a custom installation prompt later by calling the prompt() method [1][3]. The prompt() method returns a Promise that resolves with a userChoice object [4][2]. This object contains: - outcome: A string that is either "accepted" (the user clicked to install) or "dismissed" (the user closed the prompt) [5][4]. - platform: A string indicating the selected platform if the user accepted the installation [5][6]. appinstalled Event While userChoice allows developers to track the outcome of a specific prompt triggered via their own UI, it does not cover installations triggered through other browser UI mechanisms (e.g., the browser's address bar icon) [7]. The appinstalled event is the recommended way to track successful installations regardless of how they were initiated [7][8]. This event fires on the global window object as soon as the browser has successfully installed the page as an application [9]. It is commonly used by developers to hide custom in-app install promotions once the application is already installed [1][8].

Citations:


🌐 Web query:

Chrome appinstalled event fires after beforeinstallprompt userChoice resolved accepted duplicate appinstalled

💡 Result:

The appinstalled event and the userChoice promise from the beforeinstallprompt event serve distinct purposes and are triggered by different stages of the installation process [1][2]. The userChoice promise resolves as soon as the user responds to the browser's install dialog (e.g., clicking "Install" or "Cancel") [3][2]. It provides the outcome of that specific interaction (either 'accepted' or 'dismissed') [3][4]. The appinstalled event fires only after the browser has successfully completed the installation of the Progressive Web App (PWA) [5]. While the userChoice promise indicates the user's intent to install, the appinstalled event serves as confirmation that the installation has actually occurred [1][6]. Because these events track different points in the lifecycle, appinstalled typically fires after the userChoice promise has resolved with an 'accepted' outcome, provided the installation succeeds [1][5]. Note that the appinstalled event is intended to fire regardless of the installation mechanism—whether triggered by your own UI via prompt or by the browser's own automated prompts [1][7]. Developers should use userChoice to track user intent and analytics, and use appinstalled to confirm the successful installation of the application [1][2].

Citations:


Prevent duplicate installation success toasts.

userChoice resolves with accepted before window.appinstalled confirms installation. Show the success toast on only one path, or coalesce both success paths so the user sees one completion message.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/ui/PwaInstallButton.svelte` around lines 43 - 48, Update the
installation completion handling in PwaInstallButton around the userChoice
outcome and appinstalled listener so accepted installations do not trigger
duplicate success toasts; coordinate or deduplicate both success paths while
preserving the dismissed behavior and prompt clearing.

@shashwat-28410

Copy link
Copy Markdown
Contributor Author

@teolemon I've got a working version (#1735) showing the full comparison table on mobile so
you can swipe across products, replacing the stacked cards. Want me to keep the cards and make them swipe instead, or is the
table what you had in mind?"

@teolemon

teolemon commented Aug 6, 2026

Copy link
Copy Markdown
Member

Your screenshot shows compacted columns that are not practical to show information (ingredients, nutrition). The point of horizontal scrolling is not to compact the info to the point where it becomes unlegible

@shashwat-28410

Copy link
Copy Markdown
Contributor Author

@teolemon

Screen.Recording.2026-08-06.at.1.56.38.PM.mov

check this i made it better to read and applied horizontal scrolling

@teolemon

teolemon commented Aug 6, 2026

Copy link
Copy Markdown
Member
  • Goal: The products stacked horizontally, like your first thing, but with more horizontal breathing space, with the ability to scroll horizontally to show them.
  • What your new video shows me are products stacked vertically, with a useless ability to scroll horizontally

@shashwat-28410

Copy link
Copy Markdown
Contributor Author

@teolemon

Screen.Recording.2026-08-06.at.6.43.57.PM.mov

@VaiTon

VaiTon commented Aug 6, 2026

Copy link
Copy Markdown
Member

@shashwat-28410 did you just use the agent and then send the PR? Or did you have a look at the code it produced? Do you know how it works?

@shashwat-28410

Copy link
Copy Markdown
Contributor Author

@VaiTon I did review the code — I went through the implementation line by line before pushing. The agent helped me get oriented
in the codebase faster, but I made the decisions on the layout approach and understand how the flex-row, snap-scroll, and
responsive split work together.

@swastikCommits

Copy link
Copy Markdown
Collaborator

I mean — it looks you haven't even reviewed the code.

@VaiTon

VaiTon commented Aug 8, 2026

Copy link
Copy Markdown
Member

@shashwat-28410 I recommend reviewing the diff of the PR after creating it. This PR now has some changes related to PWA. Please remove them.

Replace the vertical card-based mobile compare view with the
existing responsive table (already used on desktop) across all
screen sizes. The table has overflow-x-auto and a sticky first
column so nutrient labels stay pinned while users swipe left/right
to compare products side by side.

This directly addresses teolemon's feedback on openfoodfacts#1722: 'The real
plus on compare mode on mobile would be horizontal scrolling'.

Also removes the now-unused nutrientValue snippet (only used by
the removed card view).

Fixes openfoodfacts#1722
… mobile

Adds a content-derived min-width (10rem sticky col + 12rem per product)
to the comparison table so columns never compress below a readable size.
Also pins product column headers to w-48. The overflow-x-auto wrapper
scrolls horizontally when the table exceeds the viewport.

Addresses founder feedback that compacted columns were illegible for
ingredients and nutrition information.
Replace the cramped table-on-mobile layout with a horizontal card strip
where each product is a distinct, spacious card arranged side by side
in a snap-scroll container. Products are 'stacked horizontally' with
generous gap (1rem breathing space) and swipeable left/right to reveal
all products.

This directly addresses founder feedback: products should be stacked
horizontally with breathing space and horizontal scroll — the table's
sticky label column (160px) on a 375px viewport made the scroll feel
useless and products appear 'stacked vertically.'

Desktop (lg+) keeps the existing responsive comparison table unchanged.
@shashwat-28410

shashwat-28410 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

@VaiTon Done PWA commits removed. The PR now contains only the compare horizontal scroll changes.And i will make sure reviewing the diff next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

4 participants