feat: sort by order should remember last selection and not switch #8595
Conversation
WalkthroughIntroduces journey tracking via localStorage to record recently opened items, updates sorting to prioritize tracked journeys, converts JourneySort to a controlled component, adds persistent sort preferences to TemplateList, and improves UI scrollbar and button styling. Changes
Sequence Diagram(s)sequenceDiagram
participant Router
participant useTrackRecentlyOpenedJourney
participant trackRecentlyOpenedJourney
participant localStorage
participant sortJourneys
participant JourneySort
Router->>useTrackRecentlyOpenedJourney: routeChangeComplete /journeys/{id}
useTrackRecentlyOpenedJourney->>trackRecentlyOpenedJourney: trackRecentlyOpenedJourney(journeyId)
trackRecentlyOpenedJourney->>localStorage: store {journeyId, openedAt}
Note over JourneySort: User triggers sort
JourneySort->>sortJourneys: sortJourneys(journeys, sortOrder)
sortJourneys->>localStorage: getRecentlyOpenedJourney()
localStorage-->>sortJourneys: RecentlyOpenedJourneyRecord | null
sortJourneys->>sortJourneys: isolate recent, sort others
sortJourneys-->>JourneySort: [recentlyOpened, ...sortedOthers]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 (1)
apps/journeys-admin/src/components/JourneyList/JourneySort/JourneySort.tsx (1)
118-118: Fix typo in color property.
'secondar.light'should be'secondary.light'. This typo will cause the color to not be applied correctly.Proposed fix
sx={{ backgroundColor: 'white', border: '1px solid', borderColor: 'divider', - color: 'secondar.light' + color: 'secondary.light' }}
🧹 Nitpick comments (7)
libs/journeys/ui/src/components/TemplateGallery/HeaderAndLanguageFilter/HeaderAndLanguageFilter.tsx (1)
181-186: Consider removinganchorElfrom the dependency array.The guard
anchorEl !== popperAnchorprevents unnecessary state updates, but includinganchorElin the dependency array means the effect still re-runs on everyanchorElchange. Since the DOM elementpopperAnchoris stable after mount, an empty dependency array with the guard would be more efficient.Suggested improvement
useEffect(() => { const popperAnchor = document?.getElementById('popperAnchorLayer') - if (popperAnchor != null && anchorEl !== popperAnchor) { + if (popperAnchor != null) { setAnchorEl(popperAnchor) } - }, [anchorEl]) + }, [])apps/journeys-admin/src/components/JourneyList/JourneySort/utils/trackRecentlyOpenedJourney.ts (1)
40-48: Consider adding staleness check for the recently opened journey.The
openedAttimestamp is stored but never used. If a user opens a journey, leaves for days, and returns, that journey still appears first. Consider adding an optional expiration (e.g., 24 hours) to ensure relevance.Optional staleness check
+const STALENESS_THRESHOLD_MS = 24 * 60 * 60 * 1000 // 24 hours + export function getRecentlyOpenedJourney(): RecentlyOpenedJourneyRecord | null { if (typeof window === 'undefined') return null try { const storedValue = localStorage.getItem(RECENTLY_OPENED_JOURNEY_KEY) - return parseStoredJourney(storedValue) + const record = parseStoredJourney(storedValue) + if (record == null) return null + + const isStale = Date.now() - record.openedAt > STALENESS_THRESHOLD_MS + return isStale ? null : record } catch { return null } }apps/journeys-admin/src/libs/useTrackRecentlyOpenedJourney/useTrackRecentlyOpenedJourney.ts (1)
30-30: Consider narrowing the dependency array.Using the entire
routerobject as a dependency may cause unnecessary effect re-runs. Consider depending on specific stable properties.Refined dependencies
- }, [router]) + }, [router.isReady, router.asPath, router.events])apps/journeys-admin/src/components/Editor/Slider/JourneyFlow/AnalyticsOverlaySwitch/AnalyticsOverlaySwitch.tsx (1)
38-43: Unusederrorparameter may trigger lint warnings.The parameter was renamed from
_toerrorbut remains unused. Consider either logging the error for debugging or reverting to_to indicate intentional disuse.Option A: Log the error
onError: (error) => { + console.error('Analytics fetch error:', error) enqueueSnackbar(t('Error fetching analytics'), { variant: 'error', preventDuplicate: true }) }Option B: Keep underscore convention
- onError: (error) => { + onError: (_error) => {apps/journeys-admin/src/components/JourneyList/JourneySort/JourneySort.tsx (1)
120-120: Simplify disabled prop check.The expression
disabled != null && disabledcan be simplified to justdisabledsinceundefinedandfalseare both falsy.Proposed fix
- disabled={disabled != null && disabled} + disabled={disabled}apps/journeys-admin/src/components/TemplateList/TemplateList.tsx (1)
58-62: Consider simplifying the null check.Since
sortOrderis always initialized with a value (either from localStorage or the defaultSortOrder.UPDATED_AT), thesortOrder != nullcheck is redundant. However, keeping it provides defensive coding against future changes.Optional simplification
useEffect(() => { - if (sortOrder != null && typeof window !== 'undefined') { + if (typeof window !== 'undefined') { localStorage.setItem(SORT_ORDER_STORAGE_KEY, sortOrder) } }, [sortOrder])apps/journeys-admin/src/components/JourneyList/JourneySort/utils/sortJourneys.ts (1)
16-23: Consider combining find and filter into a single pass.Currently the code iterates the array twice (
find+filter). For small arrays this is fine, but could be optimized with a single iteration usingreduce.Optional optimization using reduce
- const recentlyOpenedJourney = - recentlyOpenedJourneyId != null - ? journeys.find((j) => j.id === recentlyOpenedJourneyId) - : null - const otherJourneys = - recentlyOpenedJourneyId != null - ? journeys.filter((j) => j.id !== recentlyOpenedJourneyId) - : journeys + let recentlyOpenedJourney: Journey | null = null + const otherJourneys: Journey[] = [] + + for (const journey of journeys) { + if (recentlyOpenedJourneyId != null && journey.id === recentlyOpenedJourneyId) { + recentlyOpenedJourney = journey + } else { + otherJourneys.push(journey) + } + }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
apps/journeys-admin/pages/_app.tsxapps/journeys-admin/src/components/Editor/Slider/JourneyFlow/AnalyticsOverlaySwitch/AnalyticsOverlaySwitch.tsxapps/journeys-admin/src/components/JourneyList/JourneySort/JourneySort.tsxapps/journeys-admin/src/components/JourneyList/JourneySort/utils/sortJourneys.tsapps/journeys-admin/src/components/JourneyList/JourneySort/utils/trackRecentlyOpenedJourney.tsapps/journeys-admin/src/components/TemplateList/TemplateList.tsxapps/journeys-admin/src/libs/useTrackRecentlyOpenedJourney/useTrackRecentlyOpenedJourney.tslibs/journeys/ui/src/components/Card/WebsiteCover/WebsiteCover.tsxlibs/journeys/ui/src/components/TemplateGallery/HeaderAndLanguageFilter/HeaderAndLanguageFilter.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
apps/journeys-admin/pages/_app.tsxlibs/journeys/ui/src/components/Card/WebsiteCover/WebsiteCover.tsxapps/journeys-admin/src/libs/useTrackRecentlyOpenedJourney/useTrackRecentlyOpenedJourney.tsapps/journeys-admin/src/components/TemplateList/TemplateList.tsxapps/journeys-admin/src/components/Editor/Slider/JourneyFlow/AnalyticsOverlaySwitch/AnalyticsOverlaySwitch.tsxapps/journeys-admin/src/components/JourneyList/JourneySort/utils/sortJourneys.tslibs/journeys/ui/src/components/TemplateGallery/HeaderAndLanguageFilter/HeaderAndLanguageFilter.tsxapps/journeys-admin/src/components/JourneyList/JourneySort/utils/trackRecentlyOpenedJourney.tsapps/journeys-admin/src/components/JourneyList/JourneySort/JourneySort.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
apps/journeys-admin/pages/_app.tsxlibs/journeys/ui/src/components/Card/WebsiteCover/WebsiteCover.tsxapps/journeys-admin/src/libs/useTrackRecentlyOpenedJourney/useTrackRecentlyOpenedJourney.tsapps/journeys-admin/src/components/TemplateList/TemplateList.tsxapps/journeys-admin/src/components/Editor/Slider/JourneyFlow/AnalyticsOverlaySwitch/AnalyticsOverlaySwitch.tsxapps/journeys-admin/src/components/JourneyList/JourneySort/utils/sortJourneys.tslibs/journeys/ui/src/components/TemplateGallery/HeaderAndLanguageFilter/HeaderAndLanguageFilter.tsxapps/journeys-admin/src/components/JourneyList/JourneySort/utils/trackRecentlyOpenedJourney.tsapps/journeys-admin/src/components/JourneyList/JourneySort/JourneySort.tsx
apps/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/apps.mdc)
apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Files:
apps/journeys-admin/pages/_app.tsxapps/journeys-admin/src/libs/useTrackRecentlyOpenedJourney/useTrackRecentlyOpenedJourney.tsapps/journeys-admin/src/components/TemplateList/TemplateList.tsxapps/journeys-admin/src/components/Editor/Slider/JourneyFlow/AnalyticsOverlaySwitch/AnalyticsOverlaySwitch.tsxapps/journeys-admin/src/components/JourneyList/JourneySort/utils/sortJourneys.tsapps/journeys-admin/src/components/JourneyList/JourneySort/utils/trackRecentlyOpenedJourney.tsapps/journeys-admin/src/components/JourneyList/JourneySort/JourneySort.tsx
🧠 Learnings (6)
📚 Learning: 2025-11-11T23:22:02.196Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 8156
File: apis/api-journeys-modern/src/lib/google/googleAuth.ts:0-0
Timestamp: 2025-11-11T23:22:02.196Z
Learning: In apis/api-journeys-modern, use the validated `env` object from `../../env` instead of accessing `process.env` directly for environment variables that are defined in env.ts (e.g., GOOGLE_CLIENT_ID, GOOGLE_CLIENT_SECRET, INTEGRATION_ACCESS_KEY_ENCRYPTION_SECRET). This eliminates the need for runtime validation checks since Zod validates them at application startup.
Applied to files:
apps/journeys-admin/pages/_app.tsx
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Use `<Link>` for navigation instead of custom click handlers in Next.js.
Applied to files:
apps/journeys-admin/pages/_app.tsxapps/journeys-admin/src/libs/useTrackRecentlyOpenedJourney/useTrackRecentlyOpenedJourney.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Preserve existing contracts in components like `VideoBlock` which depend on the full provider stack, video.js, and mux metadata so autoplay, subtitles, and analytics remain intact.
Applied to files:
libs/journeys/ui/src/components/Card/WebsiteCover/WebsiteCover.tsx
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Use alphabetical order for imports and exports.
Applied to files:
apps/journeys-admin/src/components/TemplateList/TemplateList.tsxapps/journeys-admin/src/components/JourneyList/JourneySort/JourneySort.tsx
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/*.{ts,tsx} : Use alphabetical order for imports and exports
Applied to files:
apps/journeys-admin/src/components/TemplateList/TemplateList.tsx
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Tailwind utilities should stay semantically grouped—Prettier sorts the lists, but ensure responsive and state variants remain readable.
Applied to files:
apps/journeys-admin/src/components/TemplateList/TemplateList.tsx
🧬 Code graph analysis (4)
apps/journeys-admin/pages/_app.tsx (1)
apps/journeys-admin/src/libs/useTrackRecentlyOpenedJourney/useTrackRecentlyOpenedJourney.ts (1)
useTrackRecentlyOpenedJourney(6-31)
apps/journeys-admin/src/libs/useTrackRecentlyOpenedJourney/useTrackRecentlyOpenedJourney.ts (1)
apps/journeys-admin/src/components/JourneyList/JourneySort/utils/trackRecentlyOpenedJourney.ts (1)
trackRecentlyOpenedJourney(24-38)
apps/journeys-admin/src/components/TemplateList/TemplateList.tsx (1)
apps/journeys-admin/src/components/JourneyList/JourneyList.tsx (1)
JourneyListEvent(22-31)
apps/journeys-admin/src/components/Editor/Slider/JourneyFlow/AnalyticsOverlaySwitch/AnalyticsOverlaySwitch.tsx (1)
apis/api-gateway/src/logger.ts (1)
error(34-37)
⏰ 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: Analyze (javascript)
🔇 Additional comments (8)
libs/journeys/ui/src/components/TemplateGallery/HeaderAndLanguageFilter/HeaderAndLanguageFilter.tsx (1)
71-88: LGTM! Button interaction styling removed.The combination of
disableRipple,disableTouchRipple, and transparent backgrounds across all states effectively removes visual feedback. This appears intentional for a cleaner UI.libs/journeys/ui/src/components/Card/WebsiteCover/WebsiteCover.tsx (1)
149-162: Scrollbar hiding implementation is comprehensive.The cross-browser scrollbar hiding covers WebKit, Firefox, and IE/Edge appropriately.
Minor considerations:
overflowY: 'scroll'forces scroll behavior even when content fits. If'auto'was intentionally changed to'scroll', this is fine; otherwise'auto'would prevent unnecessary scroll affordance.WebkitOverflowScrollingis deprecated but still provides momentum scrolling on older iOS devices—acceptable for progressive enhancement.apps/journeys-admin/src/components/JourneyList/JourneySort/utils/trackRecentlyOpenedJourney.ts (1)
1-49: Well-structured localStorage utility with proper SSR and error handling.The implementation correctly:
- Guards against SSR environments
- Validates parsed JSON structure before use
- Handles localStorage errors gracefully
- Prevents empty/whitespace journey IDs
apps/journeys-admin/src/libs/useTrackRecentlyOpenedJourney/useTrackRecentlyOpenedJourney.ts (1)
6-30: Well-implemented tracking hook with proper cleanup.The hook correctly:
- Extracts journey IDs from both
/journeys/and/templates/routes- Handles initial route tracking when router is ready
- Cleans up the event listener on unmount
apps/journeys-admin/pages/_app.tsx (1)
21-21: LGTM!The hook is appropriately placed at the app level to track journey navigation across all routes. The integration is clean and follows React hook conventions.
Also applies to: 53-54
apps/journeys-admin/src/components/JourneyList/JourneySort/JourneySort.tsx (1)
75-79: LGTM!Converting to a controlled
RadioGroupwithvalueprop is the correct approach for maintaining the sort order state in the parent component. The fallback toSortOrder.UPDATED_ATensures a sensible default.apps/journeys-admin/src/components/TemplateList/TemplateList.tsx (1)
44-55: LGTM!Good implementation of localStorage persistence with proper SSR guards and validation against the
SortOrderenum. The lazy initializer pattern foruseStateis appropriate here.apps/journeys-admin/src/components/JourneyList/JourneySort/utils/sortJourneys.ts (1)
47-50: LGTM!The final assembly logic is clean and correctly handles both cases: when a recently opened journey exists and when it doesn't.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
apps/journeys-admin/src/components/JourneyList/JourneySort/utils/sortJourneys.ts
Show resolved
Hide resolved
…er-last-selection-and-not-switch
|
View your CI Pipeline Execution ↗ for commit 68be84f
☁️ Nx Cloud last updated this comment at |
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
This comment has been minimized.
This comment has been minimized.
irenekim21
changed the title
irenekim21
deleted the
irenekim/nes-465-sort-by-order-should-remember-last-selection-and-not-switch
branch
when user clicks on their desired journey edit, and goes back to the templates page, their most recent selected/clicked on journey will be showing up first so it's easy for user to know which journey they clicked/easy to find.
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.