fix: Thread bar shows correct origin (Network, Profile, My Spaces)#2015
Open
webguru-hypha wants to merge 1 commit intomainfrom
Open
fix: Thread bar shows correct origin (Network, Profile, My Spaces)#2015webguru-hypha wants to merge 1 commit intomainfrom
webguru-hypha wants to merge 1 commit intomainfrom
Conversation
Fixes #1614 - Navigation issue: thread bar - Breadcrumb root now reflects origin: Network, Profile, or My Spaces - Add from param to space links from Network, Profile, My Spaces pages - Persist origin via cookie when navigating between spaces - Preserve from param in DHO carousel and breadcrumb item links - Add Profile to Navigation i18n (en, fr, de) Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Contributor
📝 WalkthroughWalkthroughThis PR implements breadcrumb origin tracking that persists the navigation source (Network, Profile, or My Spaces) via cookies and context. It propagates a "from" query parameter through space cards and breadcrumb links, enabling users to navigate back to their original entry point rather than defaulting to My Spaces. Changes
Sequence DiagramsequenceDiagram
actor User as User/Browser
participant Middleware
participant CookieStore as Cookie Storage
participant BreadcrumbRoot as BreadcrumbsRootSelector
participant Components as Breadcrumb & Card Components
User->>Middleware: Navigate to space from origin (Network/Profile/My-Spaces)
Middleware->>Middleware: Detect origin from URL path/query
Middleware->>CookieStore: Set breadcrumb_origin cookie (max-age: 1 day)
Middleware-->>User: Return response with cookie
User->>BreadcrumbRoot: Mount BreadcrumbsRootSelector context provider
BreadcrumbRoot->>BreadcrumbRoot: Read from URL query or cookie
BreadcrumbRoot->>BreadcrumbRoot: Provide origin via context (useBreadcrumbFrom)
Components->>BreadcrumbRoot: Read origin using useBreadcrumbFromQuery
Components->>Components: Build link href with from parameter
Components-->>User: Render breadcrumb/card with from=origin
User->>User: Click breadcrumb or space card link
User->>Middleware: Navigate with from query parameter preserved
Middleware->>CookieStore: Update cookie if origin changed
Middleware-->>User: Navigate to target with origin context maintained
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/src/app/[lang]/dho/[id]/_components/breadcrumbs-root-selector.tsx (1)
98-116: Move cookie writes out ofuseMemoto eliminate impure side effects.The
useMemohook at line 111 callssetBreadcrumbOriginCookie(), which creates a side effect inside a pure function. The same operation is duplicated in theuseEffectat line 145. Remove the cookie write from theuseMemoblock (lines 111–113) and rely solely on theuseEffectfor this side effect. Apply this change in isolation to verify it resolves the behavior correctly before making additional refactors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`[lang]/dho/[id]/_components/breadcrumbs-root-selector.tsx around lines 98 - 116, The useMemo block that computes { rootHref, rootLabel, fromQuery } currently performs an impure side effect by calling setBreadcrumbOriginCookie(resolvedFrom, resolvedProfileSlug ?? undefined); remove that call from the useMemo so the memo remains pure (leave the logic that reads getBreadcrumbOriginFromCookie() and sets resolvedFrom/resolvedProfileSlug intact), and ensure the existing useEffect that already calls setBreadcrumbOriginCookie is the only place that writes the cookie; update references to resolvedFrom/resolvedProfileSlug inside useEffect if needed to use the memoized values from useMemo rather than repeating the write here.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/`[lang]/dho/[id]/_components/breadcrumbs-root-selector.tsx:
- Around line 61-63: The code currently casts searchParams.get('from') to
BreadcrumbOrigin without validation; add a validation step (e.g., an
isValidBreadcrumbOrigin type guard or whitelist check) and only
use/persist/forward `from` when it passes that check, otherwise treat it as
null/undefined; update all usages that currently do `const from =
searchParams.get('from') as BreadcrumbOrigin` and the later propagation sites
(the cookie/query propagation logic around the `fromQuery` and cookie set code
referenced near lines 95-97, 111-114, 138-140) to call the validator and guard
before writing to cookies or including in `fromQuery`.
In `@packages/epics/src/spaces/components/space-card.container.tsx`:
- Around line 22-25: getHref currently always appends fromParam with a "?" which
breaks URLs that already contain query params; update getHref (and usage of
baseUrl from getDhoPathAgreements) to detect whether baseUrl already contains
"?" and choose "&" instead of "?" when appending fromParam, and sanitize
fromParam (strip any leading "?" or "&") before concatenation so the produced
href is valid.
---
Nitpick comments:
In `@apps/web/src/app/`[lang]/dho/[id]/_components/breadcrumbs-root-selector.tsx:
- Around line 98-116: The useMemo block that computes { rootHref, rootLabel,
fromQuery } currently performs an impure side effect by calling
setBreadcrumbOriginCookie(resolvedFrom, resolvedProfileSlug ?? undefined);
remove that call from the useMemo so the memo remains pure (leave the logic that
reads getBreadcrumbOriginFromCookie() and sets resolvedFrom/resolvedProfileSlug
intact), and ensure the existing useEffect that already calls
setBreadcrumbOriginCookie is the only place that writes the cookie; update
references to resolvedFrom/resolvedProfileSlug inside useEffect if needed to use
the memoized values from useMemo rather than repeating the write here.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b69d7730-7b95-4923-93ac-53ee8204a6cb
📒 Files selected for processing (17)
apps/web/src/app/[lang]/dho/[id]/_components/breadcrumb-space-item.tsxapps/web/src/app/[lang]/dho/[id]/_components/breadcrumbs-root-selector.tsxapps/web/src/app/[lang]/dho/[id]/_components/breadcrumbs.tsxapps/web/src/app/[lang]/dho/[id]/_components/spaces-carousel-with-from.tsxapps/web/src/app/[lang]/dho/[id]/layout.tsxapps/web/src/app/[lang]/my-spaces/page.tsxapps/web/src/middleware.tspackages/epics/src/common/get-path-function.tspackages/epics/src/people/components/profile-member-spaces.tsxpackages/epics/src/spaces/components/breadcrumbs.tsxpackages/epics/src/spaces/components/explore-spaces.tsxpackages/epics/src/spaces/components/my-filtered-spaces.tsxpackages/epics/src/spaces/components/space-card-list.tsxpackages/epics/src/spaces/components/space-card.container.tsxpackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/fr.json
Comment on lines
+61
to
+63
| const from = searchParams.get('from') as BreadcrumbOrigin | null; | ||
| const profileSlug = searchParams.get('profileSlug'); | ||
|
|
Contributor
There was a problem hiding this comment.
Validate from before using it in cookie/query propagation.
Line 61 and Line 95 trust from via type-cast only. A crafted value (e.g. ?from=foo) can be persisted to cookie and forwarded in fromQuery, creating inconsistent breadcrumb behavior across navigation.
🔧 Proposed fix
type BreadcrumbOrigin = 'network' | 'profile' | 'my-spaces';
+const BREADCRUMB_ORIGINS: readonly BreadcrumbOrigin[] = [
+ 'network',
+ 'profile',
+ 'my-spaces',
+];
+
+function parseBreadcrumbOrigin(value: string | null): BreadcrumbOrigin | null {
+ if (!value) return null;
+ return BREADCRUMB_ORIGINS.includes(value as BreadcrumbOrigin)
+ ? (value as BreadcrumbOrigin)
+ : null;
+}
@@
- const from = searchParams.get('from') as BreadcrumbOrigin | null;
+ const from = parseBreadcrumbOrigin(searchParams.get('from'));
@@
- const from = searchParams.get('from') as BreadcrumbOrigin | null;
+ const from = parseBreadcrumbOrigin(searchParams.get('from'));Also applies to: 95-97, 111-114, 138-140
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/`[lang]/dho/[id]/_components/breadcrumbs-root-selector.tsx
around lines 61 - 63, The code currently casts searchParams.get('from') to
BreadcrumbOrigin without validation; add a validation step (e.g., an
isValidBreadcrumbOrigin type guard or whitelist check) and only
use/persist/forward `from` when it passes that check, otherwise treat it as
null/undefined; update all usages that currently do `const from =
searchParams.get('from') as BreadcrumbOrigin` and the later propagation sites
(the cookie/query propagation logic around the `fromQuery` and cookie set code
referenced near lines 95-97, 111-114, 138-140) to call the validator and guard
before writing to cookies or including in `fromQuery`.
Comment on lines
+22
to
+25
| const getHref = (slug: string) => { | ||
| const baseUrl = getDhoPathAgreements(lang, slug); | ||
| return fromParam ? `${baseUrl}?${fromParam}` : baseUrl; | ||
| }; |
Contributor
There was a problem hiding this comment.
Handle existing query strings when appending fromParam.
getHref always uses ?. If baseUrl already has query params, the result is invalid.
🔧 Proposed fix
const getHref = (slug: string) => {
const baseUrl = getDhoPathAgreements(lang, slug);
- return fromParam ? `${baseUrl}?${fromParam}` : baseUrl;
+ if (!fromParam) return baseUrl;
+ const separator = baseUrl.includes('?') ? '&' : '?';
+ return `${baseUrl}${separator}${fromParam}`;
};📝 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
| const getHref = (slug: string) => { | |
| const baseUrl = getDhoPathAgreements(lang, slug); | |
| return fromParam ? `${baseUrl}?${fromParam}` : baseUrl; | |
| }; | |
| const getHref = (slug: string) => { | |
| const baseUrl = getDhoPathAgreements(lang, slug); | |
| if (!fromParam) return baseUrl; | |
| const separator = baseUrl.includes('?') ? '&' : '?'; | |
| return `${baseUrl}${separator}${fromParam}`; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/epics/src/spaces/components/space-card.container.tsx` around lines
22 - 25, getHref currently always appends fromParam with a "?" which breaks URLs
that already contain query params; update getHref (and usage of baseUrl from
getDhoPathAgreements) to detect whether baseUrl already contains "?" and choose
"&" instead of "?" when appending fromParam, and sanitize fromParam (strip any
leading "?" or "&") before concatenation so the produced href is valid.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1614 - Navigation issue: thread bar
The breadcrumb/thread bar on space and profile pages was always showing "My Spaces" as the parent. It now correctly shows the origin based on where the user navigated from: Network, Profile, or My Space.
Changes
fromquery parameter or persisted cookiefromparam when linking to spaces from:?from=network)?from=profile&profileSlug=xxx)?from=my-spaces)fromparam, so it persists when navigating between spaces (e.g. via carousel or breadcrumb)fromparam when linking to other spacesfromparamHow it works
?from=network,?from=profile&profileSlug=xxx, or?from=my-spacesfromparam on DHO pagesfromparam from the URL (or cookie as fallback) and displays the appropriate root linkfromparam so the origin stays consistent during the sessionSummary by CodeRabbit