Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(mobile): implement collection entry list #2787

Merged
merged 2 commits into from
Feb 17, 2025
Merged

Conversation

lawvs
Copy link
Member

@lawvs lawvs commented Feb 17, 2025

Description

Fix FOL-1586

PR Type

  • Feature
  • Bugfix
  • Hotfix
  • Other (please describe):

Screenshots (if UI change)

Demo Video (if new feature)

Linked Issues

Additional context

Changelog

  • I have updated the changelog/next.md with my changes.

Copy link

linear bot commented Feb 17, 2025

@follow-reviewer-bot
Copy link

Suggested PR Title:

feat(collection): improve handling in feeds and hooks

Change Summary:
Enhance collection handling in feed screens and hooks. Added collection check in payload; integrated router for seamless navigation to collection feeds; improved entry list retrieval for collections with new hooks.

Code Review:

Code Review

apps/mobile/src/screens/(stack)/feeds/[feedId]/index.tsx

  • Lines 13-16: The variable view is passed to useCollectionEntryList with a fallback value of 0 (view ?? 0). However, the fallback logic for view seems inconsistent with the check in apps/mobile/src/store/entry/store.ts where view === undefined triggers an error. Consider aligning the behavior and ensuring that view is properly validated or has a consistent fallback throughout the app to avoid unexpected bugs.

apps/mobile/src/store/collection/hooks.ts

  • Lines 5-40: The useCallback hooks used in the selectors (e.g., useCollectionEntry and useCollectionEntryList) seem unnecessary because the useCollectionStore function already optimizes re-renders. useCallback here adds extra complexity without tangible benefit since the selector functions depend solely on their arguments, which are primitive (e.g., entryId, view). Consider removing useCallback to simplify the code while still achieving the same level of optimization.

apps/mobile/src/store/entry/store.ts

  • Line 310: In the error log, "view is required for collection", the use of console.error is appropriate for debugging but might benefit from more structured error handling. Consider using a logging service or a custom error object if the application has an established pattern for logging more detailed error contexts.

No further issues were identified in other files. The changes appear to be well-scoped and functional enhancements.


Summary

  • Address the inconsistency in view fallback logic across files.
  • Simplify useCallback usage in apps/mobile/src/store/collection/hooks.ts to reduce unnecessary complexity.
  • Consider improving error handling for the console.error case in apps/mobile/src/store/entry/store.ts.

Copy link

vercel bot commented Feb 17, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
follow ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 17, 2025 4:51pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
follow-external-ssr ⬜️ Ignored (Inspect) Visit Preview Feb 17, 2025 4:51pm

@lawvs lawvs marked this pull request as ready for review February 17, 2025 16:49
@lawvs lawvs merged commit c251aea into dev Feb 17, 2025
11 checks passed
@lawvs lawvs deleted the feat/collection-list branch February 17, 2025 18:04
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.

1 participant