Skip to content

Conversation

@Bekiboo
Copy link
Collaborator

@Bekiboo Bekiboo commented Jan 7, 2026

Description of change

Fixed infinite scrolling for mobile on pictique

Depends on: PR #656
Please review after PR #656 is merged.

Issue Number

Closes #659

Type of change

  • Fix (a change which fixes an issue)

How the change has been tested

Tested on Chrome, in dev tools, mobile view

Change checklist

  • I have ensured that the CI Checks pass locally
  • I have removed any unnecessary logic
  • My code is well documented
  • I have signed my commits
  • My code follows the pattern of the application
  • I have self reviewed my code

Summary by CodeRabbit

  • New Features

    • Improved infinite-scroll behavior with explicit scroll container handling and a clearer loading block (spinner + descriptive text).
    • Desktop post list padding adjusted for better spacing on small screens.
  • Bug Fixes

    • More reliable load-more triggering to reduce missed fetches.
  • Style

    • Avatar styling refinements for consistent sizing and image fit.
  • Chores

    • Minor message-preview formatting cleanup.

✏️ Tip: You can customize this high-level summary in your review settings.

@Bekiboo Bekiboo self-assigned this Jan 7, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 7, 2026

📝 Walkthrough

Walkthrough

Updated avatar styling (added classes, removed runtime inspect) and adjusted the home page infinite-scroll IntersectionObserver to use the sentinel's parent as root with a 200px rootMargin, added a load-more console log, adjusted list padding, and replaced the simple loading text with a spinner block. Minor formatting change on messages page.

Changes

Cohort / File(s) Summary
Avatar Styling
platforms/pictique/src/lib/ui/Avatar/Avatar.svelte
Added CSS classes shrink-0, aspect-square, object-cover to the avatar element; removed the $inspect(img) call.
Home Page — Infinite Scroll & UI
platforms/pictique/src/routes/(protected)/home/+page.svelte
IntersectionObserver now sets root to the sentinel's parent (scrollContainer) and uses rootMargin: '200px'; added console.log when loadMoreFeed triggers; adjusted post list padding (pb-24 on small, removed on md); replaced inline loading text with a spinner + descriptive block; sentinel remains for infinite scroll.
Messages Page — Formatting
platforms/pictique/src/routes/(protected)/messages/+page.svelte
Reformatted the conditional that trims $$system-message$$ prefix from previewText into a multi-line if-statement without behavioral changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • coodos
  • sosweetham

Poem

🐰 I nudge the feed, a tiny hop and spin,
Avatar snug, no inspect to begin,
Sentinel watches from its parent root,
Spinning loader hums—more posts en route,
I nibble bytes and watch the scroll leap in.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the fix as addressing infinite scrolling on mobile for Pictique, matching the primary changes in the PR.
Description check ✅ Passed The PR description includes all required template sections: issue number, type of change, testing details, and completed checklist items.
Linked Issues check ✅ Passed The PR addresses issue #659 by implementing IntersectionObserver root configuration for mobile scrolling and updating mobile UI, fixing infinite scrolling functionality on mobile devices.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the mobile infinite scrolling issue: Avatar styling cleanup, IntersectionObserver mobile support, mobile UI padding, loading indicator updates, and message formatting.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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 and usage tips.

@Bekiboo Bekiboo marked this pull request as ready for review January 7, 2026 07:35
@Bekiboo Bekiboo requested a review from coodos as a code owner January 7, 2026 07:35
@Bekiboo Bekiboo marked this pull request as draft January 7, 2026 07:35
@Bekiboo Bekiboo marked this pull request as ready for review January 7, 2026 07:37
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
platforms/pictique/src/routes/(protected)/messages/+page.svelte (2)

21-21: Use browser-compatible timeout type.

NodeJS.Timeout is a Node.js-specific type. In browser code, use ReturnType<typeof setTimeout> or simply number for the timer ID.

🔎 Proposed fix
-	let debounceTimer: NodeJS.Timeout;
+	let debounceTimer: ReturnType<typeof setTimeout>;

44-44: Remove debug logging.

This console.log statement should be removed before merging to production.

🔎 Proposed fix
-			console.log('Raw chat data from API:', data.chats);
platforms/pictique/src/lib/fragments/Header/Header.svelte (1)

107-125: Stale component documentation.

The JSDoc comment references props (variant, heading, callback) that are no longer accepted by the component. Update or remove this documentation to avoid confusion.

🤖 Fix all issues with AI agents
In @platforms/pictique/src/routes/(protected)/messages/+page.svelte:
- Line 319: The existing overlay div uses the deprecated Tailwind v3 opacity
utility "bg-opacity-50" together with "bg-black"; replace that with the v4 slash
syntax by removing "bg-opacity-50" and converting the background class to
"bg-black/50" in the div that currently contains the class string "bg-opacity-50
... bg-black" (around the overlay div) and make the same replacement for the
second occurrence referenced at line 430 so both overlays use "bg-black/50".
🧹 Nitpick comments (8)
platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte (1)

55-59: Static break heights won't update on resize.

The breaks configuration uses window.innerHeight captured at mount time. If the user rotates their device or resizes the browser while the drawer is open, the break heights become stale. For a short-lived drawer this is likely acceptable, but consider recalculating on orientation change if issues arise.

platforms/pictique/src/routes/(protected)/messages/+page.svelte (1)

111-118: Redundant API call for user data.

loadMessages() already fetches /api/users and sets currentUserId. The subsequent call on line 116 duplicates this work.

🔎 Proposed refactor

Consider having loadMessages return userData or extracting the user fetch to a separate function called once:

 	onMount(async () => {
 		try {
-			await loadMessages();
-
-			// Get current user first
-			const { data: userData } = await apiClient.get('/api/users');
-			currentUserId = userData.id;
-			profile = userData; // Set profile data
+			// Get current user first
+			const { data: userData } = await apiClient.get('/api/users');
+			currentUserId = userData.id;
+			profile = userData;
+
+			await loadMessages();

Then remove the duplicate fetch from loadMessages, or pass the userId to it.

platforms/pictique/src/routes/(protected)/messages/+layout.svelte (1)

9-11: Empty RightPanel placeholder.

The RightPanel snippet renders an empty <div>, which reserves space but provides no content. If this is a placeholder for future functionality, consider adding a TODO comment. Otherwise, remove the snippet to let MainPanel handle the absence of a right panel gracefully.

platforms/pictique/src/routes/(protected)/discover/+page.svelte (1)

45-47: Remove debug logging.

The $effect block with console.log($searchResults) appears to be leftover debug code. Consider removing it before merging.

🔎 Proposed fix
-	$effect(() => {
-		console.log($searchResults);
-	});
platforms/pictique/src/lib/fragments/MainPanel/MainPanel.svelte (1)

27-27: Redundant optional chaining.

RightPanel?.() uses optional chaining, but the call is already inside an {#if RightPanel} guard, making the ?. unnecessary.

🔎 Proposed fix
-			{@render RightPanel?.()}
+			{@render RightPanel()}
platforms/pictique/src/lib/fragments/Header/Header.svelte (1)

11-29: Consider using $derived.by for heading.

The heading variable is computed purely from route, so using $derived.by instead of $state + $effect would be more idiomatic and avoid the intermediate mutable state.

🔎 Proposed fix
-	let heading = $state('');
-
-	$effect(() => {
-		if (route.includes('home')) {
-			heading = 'Feed';
-		} else if (route.includes('/discover')) {
-			heading = 'Search';
-		} else if (route.includes('/post/audience')) {
-			heading = 'Audience';
-		} else if (route.includes('/post')) {
-			heading = 'Upload photo';
-		} else if (route === '/messages') {
-			heading = 'Messages';
-		} else if (route.includes('/settings')) {
-			heading = 'Settings';
-		} else if (route.includes('/profile')) {
-			heading = 'Profile';
-		}
-	});
+	let heading = $derived.by(() => {
+		if (route.includes('home')) return 'Feed';
+		if (route.includes('/discover')) return 'Search';
+		if (route.includes('/post/audience')) return 'Audience';
+		if (route.includes('/post')) return 'Upload photo';
+		if (route === '/messages') return 'Messages';
+		if (route.includes('/settings')) return 'Settings';
+		if (route.includes('/profile')) return 'Profile';
+		return '';
+	});
platforms/pictique/src/routes/(protected)/home/+page.svelte (2)

45-52: Remove debug logging before merge.

The console.log statements for debugging the intersection observer and loadMoreFeed trigger should be removed before merging to production.

🔎 Proposed fix
 				if (entry.isIntersecting) {
 					const hasMorePosts = get(hasMore);
 					const loading = get(isLoading);
 					const loadingMore = get(isLoadingMore);

-					console.log('Sentinel intersecting:', {
-						hasMorePosts,
-						loading,
-						loadingMore
-					});
-
 					if (hasMorePosts && !loading && !loadingMore) {
-						console.log('Triggering loadMoreFeed');
 						loadMoreFeed();
 					}
 				}

327-353: Consider moving SingleComment snippet inside the component.

The SingleComment snippet is defined outside the <MainPanel> block but rendered inside both RightPanel and Drawer. While this works in Svelte 5, co-locating it near its usage or extracting it to a separate component would improve readability.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7af900a and 951efe9.

📒 Files selected for processing (14)
  • platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte
  • platforms/pictique/src/lib/fragments/Header/Header.svelte
  • platforms/pictique/src/lib/fragments/MainPanel/MainPanel.svelte
  • platforms/pictique/src/lib/fragments/RightAside/RightAside.svelte
  • platforms/pictique/src/lib/ui/Avatar/Avatar.svelte
  • platforms/pictique/src/routes/(protected)/+layout.svelte
  • platforms/pictique/src/routes/(protected)/discover/+page.svelte
  • platforms/pictique/src/routes/(protected)/home/+page.svelte
  • platforms/pictique/src/routes/(protected)/messages/+layout.svelte
  • platforms/pictique/src/routes/(protected)/messages/+page.svelte
  • platforms/pictique/src/routes/(protected)/post/+layout.svelte
  • platforms/pictique/src/routes/(protected)/profile/+layout.svelte
  • platforms/pictique/src/routes/(protected)/settings/+layout.svelte
  • platforms/pictique/src/routes/+layout.svelte
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.

Applied to files:

  • platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte
  • platforms/pictique/src/routes/(protected)/post/+layout.svelte
  • platforms/pictique/src/lib/fragments/Header/Header.svelte
  • platforms/pictique/src/routes/(protected)/settings/+layout.svelte
  • platforms/pictique/src/routes/(protected)/profile/+layout.svelte
  • platforms/pictique/src/routes/(protected)/messages/+layout.svelte
  • platforms/pictique/src/routes/(protected)/messages/+page.svelte
  • platforms/pictique/src/routes/+layout.svelte
  • platforms/pictique/src/routes/(protected)/discover/+page.svelte
  • platforms/pictique/src/routes/(protected)/+layout.svelte
  • platforms/pictique/src/routes/(protected)/home/+page.svelte
  • platforms/pictique/src/lib/ui/Avatar/Avatar.svelte
  • platforms/pictique/src/lib/fragments/MainPanel/MainPanel.svelte
  • platforms/pictique/src/lib/fragments/RightAside/RightAside.svelte
📚 Learning: 2025-05-08T09:02:43.311Z
Learnt from: pixel-punk-20
Repo: MetaState-Prototype-Project/prototype PR: 121
File: platforms/metagram/src/stories/Button.svelte:0-0
Timestamp: 2025-05-08T09:02:43.311Z
Learning: The project uses Svelte 5, which introduces the new "runes" API including $props() for handling component props instead of the export let syntax from Svelte 3/4.

Applied to files:

  • platforms/pictique/src/lib/fragments/Header/Header.svelte
📚 Learning: 2025-05-08T09:02:43.311Z
Learnt from: pixel-punk-20
Repo: MetaState-Prototype-Project/prototype PR: 121
File: platforms/metagram/src/stories/Button.svelte:0-0
Timestamp: 2025-05-08T09:02:43.311Z
Learning: The project uses Svelte 5, which uses the new "runes" API including $props() for handling component props instead of the export let syntax from Svelte 3/4.

Applied to files:

  • platforms/pictique/src/lib/fragments/Header/Header.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (14)
platforms/pictique/src/lib/ui/Avatar/Avatar.svelte (1)

24-24: LGTM!

Good additions: shrink-0 prevents flex shrinking, aspect-square ensures consistent proportions, and object-cover handles image scaling properly. Removing the debug $inspect call is appropriate cleanup.

platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte (2)

61-62: Good addition of close callbacks.

Wiring onBackdropTap to dismiss and onWillDismiss to call onClose?.() ensures parent components are properly notified when the drawer closes, enabling coordinated state cleanup.


77-83: Enabling scroll within the pane supports the infinite scrolling fix.

Adding overflow-y: scroll to the .pane global style allows content to scroll within the drawer on mobile, which aligns with the PR's objective to fix infinite scrolling.

platforms/pictique/src/lib/fragments/RightAside/RightAside.svelte (1)

6-22: LGTM!

Good refactor: switching to the idiomatic children prop, making header optional, and applying responsive classes (hidden md:flex) properly constrains this panel to desktop views. The scroll behavior with hidden scrollbar is well-implemented.

platforms/pictique/src/routes/(protected)/messages/+page.svelte (1)

268-316: Good implementation of loading states and pagination UI.

The loading spinner, message rendering, and "Load More" button with status indicators provide clear feedback to users. The pagination logic is straightforward and maintains good UX.

platforms/pictique/src/routes/(protected)/post/+layout.svelte (1)

7-13: LGTM!

This layout follows the established MainPanel pattern used across other protected routes. The empty RightPanel placeholder maintains layout consistency.

platforms/pictique/src/routes/+layout.svelte (1)

46-50: LGTM!

Good cleanup: replacing the arbitrary h-[100dvh] with the standard h-dvh utility is cleaner and more maintainable.

platforms/pictique/src/routes/(protected)/settings/+layout.svelte (1)

7-13: LGTM!

Consistent with the MainPanel pattern applied across other protected routes in this PR.

platforms/pictique/src/routes/(protected)/profile/+layout.svelte (1)

1-9: LGTM!

Clean and minimal layout wrapper that follows the established MainPanel pattern used across other protected routes.

platforms/pictique/src/lib/fragments/MainPanel/MainPanel.svelte (1)

14-22: Good scroll container setup for infinite scroll fix.

The overflow-y-auto on the section element establishes a proper scroll container, which is essential for the mobile infinite scroll fix. The IntersectionObserver in home/+page.svelte can now correctly detect the sentinel within this scrollable area.

platforms/pictique/src/routes/(protected)/+layout.svelte (1)

43-55: LGTM!

The layout correctly delegates MainPanel wrapping to child routes and conditionally hides BottomNav on individual message routes.

platforms/pictique/src/routes/(protected)/home/+page.svelte (3)

33-71: Good fix for mobile infinite scroll.

Setting the IntersectionObserver's root to scrollContainer (the scrollable parent) is the correct fix. Previously, the observer likely used the viewport as the root, which doesn't work when the feed is inside a scrollable container that isn't the viewport itself. The rootMargin: '200px' provides a reasonable buffer to trigger loading before the user reaches the end.


131-131: Verify h-screen class necessity.

The ul uses h-screen but the parent MainPanel section already has overflow-y-auto and md:h-dvh. This may cause unexpected behavior on mobile where h-screen could exceed the container height. Consider using h-full or removing the explicit height to let the list grow naturally within the scrollable container.


104-121: Good race condition handling.

The pattern of capturing targetPostId and checking it against $activePostId in .catch() and .finally() callbacks correctly prevents stale state updates when the user switches posts quickly.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
platforms/pictique/src/routes/(protected)/messages/+page.svelte (2)

44-44: Remove debug console.log statement.

This debug logging statement should be removed before merging to production.

🧹 Proposed fix
-			console.log('Raw chat data from API:', data.chats);

199-200: Bug: Group name only uses first selected member instead of all members.

The filter condition m.id === selectedMembers[0] only matches the first selected member. This should filter for all selected members to build the group name correctly.

🐛 Proposed fix
-			const groupMembers = allMembers.filter((m) => m.id === selectedMembers[0]);
+			const groupMembers = allMembers.filter((m) => selectedMembers.includes(m.id));
🧹 Nitpick comments (5)
platforms/pictique/src/routes/(protected)/messages/+page.svelte (5)

21-21: Memory leak: debounce timer not cleared on unmount.

The debounceTimer is set in handleSearch but never cleared when the component unmounts. This can cause memory leaks and potentially execute stale callbacks after navigation.

♻️ Proposed fix using onDestroy
 import { onMount } from 'svelte';
+import { onDestroy } from 'svelte';
 import { heading } from '../../store';

Then add cleanup:

 let debounceTimer: NodeJS.Timeout;
+
+onDestroy(() => {
+  clearTimeout(debounceTimer);
+});

41-42: User data fetched redundantly on every pagination call.

/api/users is called inside loadMessages which runs on every page load including pagination (append = true). Consider caching currentUserId and userData after the first fetch or moving this call to onMount only.


211-218: Redundant conditional branching.

Both branches execute goto('/messages'), making the if/else unnecessary.

♻️ Proposed simplification
-		// Navigate to the new chat instead of hard refresh
-		if (selectedMembers.length === 1) {
-			// For direct messages, we need to find the chat ID
-			// For now, redirect to messages and let the user click on the new chat
-			goto('/messages');
-		} else {
-			// For group chats, redirect to messages
-			goto('/messages');
-		}
+		// Navigate to messages page - ideally navigate directly to the new chat
+		// TODO: Get chat ID from API response and navigate to /messages/{chatId}
+		goto('/messages');

246-249: Potential race condition: goto before await loadMessages.

goto('/messages') may trigger navigation and component unmount before loadMessages completes. Since you're already on /messages, the goto is likely a no-op, but the intent is unclear. Consider removing the redundant goto or reordering.

♻️ Proposed fix
-		// Navigate to messages and refresh the feed
-		goto('/messages');
-		// Refresh the messages to show the newly created group
-		await loadMessages(1, false);
+		// Refresh the messages to show the newly created group
+		await loadMessages(1, false);

375-378: Unnecessary return; statement in event handler.

The return; after toggleMemberSelection serves no purpose and can be removed.

♻️ Proposed fix
 onchange={(e: Event) => {
   toggleMemberSelection(member.id);
-  return;
 }}

Apply the same fix at line 519-522.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 951efe9 and 68cb261.

📒 Files selected for processing (1)
  • platforms/pictique/src/routes/(protected)/messages/+page.svelte
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.

Applied to files:

  • platforms/pictique/src/routes/(protected)/messages/+page.svelte
📚 Learning: 2025-05-08T08:47:11.295Z
Learnt from: pixel-punk-20
Repo: MetaState-Prototype-Project/prototype PR: 121
File: platforms/metagram/src/app.css:1-1
Timestamp: 2025-05-08T08:47:11.295Z
Learning: In Tailwind CSS v4, the traditional `tailwind base` directive is no longer available and should be replaced with `import "tailwindcss/preflight"` instead. The directive system has been changed in v4 to use direct imports.

Applied to files:

  • platforms/pictique/src/routes/(protected)/messages/+page.svelte
📚 Learning: 2025-05-08T08:47:11.295Z
Learnt from: pixel-punk-20
Repo: MetaState-Prototype-Project/prototype PR: 121
File: platforms/metagram/src/app.css:1-1
Timestamp: 2025-05-08T08:47:11.295Z
Learning: In Tailwind CSS v4, the traditional directive system has been removed. The `tailwind base` directive should be replaced with `import "tailwindcss/preflight";` for just base styles, or `import "tailwindcss";` to include all layers (preflight, components, and utilities).

Applied to files:

  • platforms/pictique/src/routes/(protected)/messages/+page.svelte
📚 Learning: 2025-08-08T04:15:35.098Z
Learnt from: sosweetham
Repo: MetaState-Prototype-Project/prototype PR: 279
File: infrastructure/control-panel/src/app.css:0-0
Timestamp: 2025-08-08T04:15:35.098Z
Learning: The MetaState-Prototype-Project uses Tailwind CSS v3, where the theme directive is valid and supported. The theme block should not be changed to :root or layer base in this project.

Applied to files:

  • platforms/pictique/src/routes/(protected)/messages/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (4)
platforms/pictique/src/routes/(protected)/messages/+page.svelte (4)

103-112: Missing infinite scroll implementation - only "Load More" button exists.

The PR objective is to fix infinite scrolling on mobile, but this implementation uses a manual "Load More" button (loadMoreMessages is triggered by button click at line 299). True infinite scroll requires detecting when the user scrolls near the bottom of the list.

Consider using IntersectionObserver on a sentinel element at the end of the list to trigger loadMoreMessages automatically.

♻️ Example infinite scroll implementation
<script lang="ts">
  // Add a reference for the sentinel element
  let sentinelRef: HTMLDivElement;
  
  onMount(async () => {
    // ... existing code ...
    
    // Set up IntersectionObserver for infinite scroll
    const observer = new IntersectionObserver(
      (entries) => {
        if (entries[0].isIntersecting && hasMorePages && !isLoadingMore) {
          loadMoreMessages();
        }
      },
      { rootMargin: '100px' }
    );
    
    if (sentinelRef) {
      observer.observe(sentinelRef);
    }
    
    return () => observer.disconnect();
  });
</script>

<!-- In template, after message list -->
{#if hasMorePages}
  <div bind:this={sentinelRef} class="h-1" aria-hidden="true"></div>
{/if}

Was the "Load More" button intentional as an alternative to infinite scroll, or should scroll-based loading be added?


285-285: Verify: Inverted unread prop logic.

unread={!message.unread} inverts the unread status. If message.unread is true (meaning unread), this passes false to the Message component. This appears counterintuitive—verify this is intentional and not a bug.


322-322: LGTM: Tailwind v4 opacity syntax correctly applied.

The bg-black/50 syntax is the correct Tailwind v4 approach, addressing the previous review comment about deprecated bg-opacity-50.


431-604: LGTM: New Group Modal implementation.

The modal is well-structured with:

  • Proper Tailwind v4 opacity syntax (bg-black/50)
  • Accessible close buttons with aria-label
  • Form validation (button disabled when conditions not met)
  • Selected members display with removal capability

hasMorePosts,
loading,
loadingMore
});
Copy link
Member

Choose a reason for hiding this comment

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

we dont need the logs

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @platforms/pictique/src/routes/(protected)/home/+page.svelte:
- Line 46: Remove the debug console.log call that prints 'Triggering
loadMoreFeed' from the +page.svelte code (delete the line containing
console.log('Triggering loadMoreFeed');), ensuring no leftover debug logging
remains in the loadMoreFeed/event handler block so production code has no stray
console output.
- Around line 33-35: The sentinel function uses node.parentElement as the scroll
container which is correct; remove the debug log by deleting the
console.log('Triggering loadMoreFeed'); call inside the IntersectionObserver
callback (refer to the sentinel function and its observer callback) and keep the
observer root set to the detected scrollContainer with the 200px rootMargin;
after removing the log, test infinite scrolling on real mobile devices and
across multiple browsers to confirm the behavior (not just DevTools mobile
emulation).
🧹 Nitpick comments (1)
platforms/pictique/src/routes/(protected)/home/+page.svelte (1)

232-237: Consider extracting the loading spinner into a reusable component.

The updated loading UI with spinner is a nice improvement. However, this loading pattern may be used elsewhere in the application.

♻️ Optional refactor suggestion

Create a reusable LoadingSpinner.svelte component:

<!-- lib/ui/LoadingSpinner.svelte -->
<script lang="ts">
	let { message = 'Loading...', size = 'md' }: { message?: string; size?: 'sm' | 'md' | 'lg' } = $props();
	
	const sizeClasses = {
		sm: 'h-4 w-4',
		md: 'h-8 w-8',
		lg: 'h-12 w-12'
	};
</script>

<div class="flex flex-col items-center justify-center py-8">
	<div class="mb-3 {sizeClasses[size]} animate-spin rounded-full border-2 border-t-transparent"></div>
	<p class="text-base font-medium text-gray-700">{message}</p>
</div>

Then use it in the page:

 {#if $isLoadingMore}
-	<li class="my-8 flex flex-col items-center justify-center py-8">
-		<div
-			class="mb-3 h-8 w-8 animate-spin rounded-full border-2 border-t-transparent"
-		></div>
-		<p class="text-base font-medium text-gray-700">Loading more posts...</p>
-	</li>
+	<li class="my-8">
+		<LoadingSpinner message="Loading more posts..." />
+	</li>
 {/if}
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b1f3beb and 2d15433.

📒 Files selected for processing (1)
  • platforms/pictique/src/routes/(protected)/home/+page.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.

Applied to files:

  • platforms/pictique/src/routes/(protected)/home/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (1)
platforms/pictique/src/routes/(protected)/home/+page.svelte (1)

125-125: Mobile padding adjustment is appropriate.

The pb-24 on mobile ensures the sentinel element can enter the viewport and trigger infinite scrolling, while md:pb-0 removes unnecessary padding on desktop. This is a key part of the mobile fix.

Comment on lines 33 to +35
const sentinel = (node: HTMLElement) => {
// The sentinel is a direct child of the scrollable ul
const scrollContainer = node.parentElement;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the file at the specified lines to verify the scroll container logic
cat -n platforms/pictique/src/routes/\(protected\)/home/+page.svelte | sed -n '30,60p'

Repository: MetaState-Prototype-Project/prototype

Length of output: 1058


🏁 Script executed:

# Search for IntersectionObserver patterns across the codebase
fd -e svelte -e ts -e js | xargs rg -n 'IntersectionObserver|console\.log' --type-list | head -50

Repository: MetaState-Prototype-Project/prototype

Length of output: 470


🏁 Script executed:

# More targeted search for console.log in the specific file
rg -n 'console\.log' platforms/pictique/src/routes/\(protected\)/home/+page.svelte

Repository: MetaState-Prototype-Project/prototype

Length of output: 212


🏁 Script executed:

# Search for other IntersectionObserver usages in the codebase
rg -n 'IntersectionObserver' --type svelte --type ts --type js

Repository: MetaState-Prototype-Project/prototype

Length of output: 111


🏁 Script executed:

# Search for other IntersectionObserver usages without specifying file type
rg -n 'IntersectionObserver'

Repository: MetaState-Prototype-Project/prototype

Length of output: 188


Scroll container detection is correct, but remove the debug log.

Using the sentinel's parent element as the IntersectionObserver root with a 200px rootMargin should fix mobile infinite scrolling. However, the console.log at line 46 needs to be removed per past feedback about unnecessary logging.

Remove the console.log('Triggering loadMoreFeed'); statement at line 46.

Also verify that infinite scrolling works correctly on actual mobile devices (not just DevTools mobile view) and various browsers.

🤖 Prompt for AI Agents
In @platforms/pictique/src/routes/(protected)/home/+page.svelte around lines 33
- 35, The sentinel function uses node.parentElement as the scroll container
which is correct; remove the debug log by deleting the console.log('Triggering
loadMoreFeed'); call inside the IntersectionObserver callback (refer to the
sentinel function and its observer callback) and keep the observer root set to
the detected scrollContainer with the 200px rootMargin; after removing the log,
test infinite scrolling on real mobile devices and across multiple browsers to
confirm the behavior (not just DevTools mobile emulation).

const loadingMore = get(isLoadingMore);
if (hasMorePosts && !loading && !loadingMore) {
console.log('Triggering loadMoreFeed');
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove the debug console.log.

This console.log statement should be removed before merging.

🧹 Proposed fix
 						if (hasMorePosts && !loading && !loadingMore) {
-							console.log('Triggering loadMoreFeed');
 							loadMoreFeed();
 						}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.log('Triggering loadMoreFeed');
if (hasMorePosts && !loading && !loadingMore) {
loadMoreFeed();
}
🤖 Prompt for AI Agents
In @platforms/pictique/src/routes/(protected)/home/+page.svelte at line 46,
Remove the debug console.log call that prints 'Triggering loadMoreFeed' from the
+page.svelte code (delete the line containing console.log('Triggering
loadMoreFeed');), ensuring no leftover debug logging remains in the
loadMoreFeed/event handler block so production code has no stray console output.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [Pictique] Infinite scrolling doesn't work on mobile

3 participants