fix: pressist search state inbox #873
Conversation
WalkthroughThe changes update the routing and search functionality within the inbox. In the conversation component, the route navigation now uses an object with a pathname and a query—where the query is extracted using a new utility function. The search feature has been enhanced by modifying the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as Search Component
participant H as useSearch Hook
participant R as Router
U->>S: Enter search text
S->>H: Trigger onSearch event
H->>H: Update searchText state
H->>R: Debounced update of query parameters
R-->>H: Reflect updated search query
Note over H: useEffect syncs searchText with URL
sequenceDiagram
participant U as User
participant C as Conversation Component
participant Q as extractQueryParamsUrl
participant R as Router
U->>C: Click on conversation
C->>Q: Extract query parameters from current URL
Q-->>C: Return query string
C->>R: Call push({ pathname, query })
Assessment against linked issues
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/utils/URL.ts (1)
63-70: Consider handling invalid or malformed URLs.
IffullUrlis invalid,new URL(fullUrl)will throw. You might want to wrap this in a try-catch block to gracefully handle that scenario, especially if the function is used in contexts wherefullUrlmight be unknown or user-provided.export const extractQueryParamsUrl = (fullUrl: string): string => { - const extractedQueryParams = qs.parse(new URL(fullUrl).search, { - ignoreQueryPrefix: true, - }); - return qs.stringify(extractedQueryParams); + try { + const extractedQueryParams = qs.parse(new URL(fullUrl).search, { + ignoreQueryPrefix: true, + }); + return qs.stringify(extractedQueryParams); + } catch (error) { + // handle or log error, returning empty string or fallback + return ""; + } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/components/inbox/components/ConversationsList.tsx(2 hunks)frontend/src/components/inbox/index.tsx(3 hunks)frontend/src/hooks/useSearch.tsx(3 hunks)frontend/src/utils/URL.ts(2 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
frontend/src/components/inbox/components/ConversationsList.tsx (1)
frontend/src/utils/URL.ts (1)
extractQueryParamsUrl(64-70)
frontend/src/components/inbox/index.tsx (1)
frontend/src/hooks/useSearch.tsx (1)
useSearch(55-109)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Frontend Tests
- GitHub Check: API-Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
frontend/src/utils/URL.ts (1)
9-9: Good use of theqslibrary.
The import is straightforward and helps parse query strings reliably.frontend/src/components/inbox/components/ConversationsList.tsx (2)
23-23: Import statement aligns well with usage.
No issues with the newly importedextractQueryParamsUrlfunction. This helps keep the query parsing logic centralized.
67-71:❓ Verification inconclusive
Watch out for potential SSR contexts.
Usingwindow.location.hrefmay not work in server-side rendering contexts. If this component ever needs to render on the server, considerrouter.asPathor storing the query string in a more SSR-friendly manner.Would you like to confirm if this code runs exclusively in the browser? If needed, we can investigate references to this component’s usage by searching the codebase:
If you discover it is server-rendered, you may need to insert guards or use Next.js router properties.
🏁 Script executed:
#!/bin/bash # Searching for usage of SubscribersList component fd SubscribersList.tsx --exec rg 'SubscribersList'Length of output: 50
Manual Verification Required: Confirm Client-Only Usage for Query Extraction
The code in
frontend/src/components/inbox/components/ConversationsList.tsx(lines 67–71) currently useswindow.location.hreffor extracting query parameters. However, our attempt to locate references to the corresponding subscriber component (via searching for “SubscribersList”) returned no output, which leaves some uncertainty about whether this code is strictly executed on the client side.
- Action Needed: Please manually verify that
ConversationsList(or any related components) is exclusively used in browser contexts.- If SSR Usage Is Possible: Consider replacing
window.location.hrefwith an SSR-safe alternative likerouter.asPathor introducing a safeguard to ensure the code executes only in the browser.frontend/src/components/inbox/index.tsx (3)
2-2: License year updated.
No code concerns here.
29-29: Addition ofsearchTextis consistent.
The extended destructuring fromuseSearchaligns with the usage pattern for controlling the search state.
51-51: Bindingvalue={searchText}ensures the search box reflects the URL state.
This is a good enhancement for synchronizing the UI with query parameters.frontend/src/hooks/useSearch.tsx (4)
2-2: License year update only.
No functional impact or concerns.
10-11: New imports are relevant to the router-based search feature.
These additions are appropriate for handling side effects and user input.
55-87: Enhanced synchronization with router query.
Initializing and syncingsearchTextwithrouter.query.searchis a solid approach, ensuring the UI remains in sync with the URL. The debouncedupdateQueryParamsfunction helps prevent excessive router updates, which is beneficial for performance.
96-96: ReturningsearchTextaids controlled components.
ExposingsearchTextmakes the search logic reusable and consistent throughout the consumer components.
|
@coderabbitai Open a follow-up GitHub issue to address the flickering search results in the inbox page |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@yassinedorbozgithub I've created a follow-up GitHub issue to address the flickering search results in the inbox page. The issue includes:
Let me know if you'd like me to modify the issue with any additional details. 🏁 Script executed: Length of output: 1149 @yassinedorbozgithub I've successfully created a GitHub issue to address the flickering search results in the inbox page. The issue is available at: The issue has been assigned to you and includes all the necessary details about the flickering problem, context related to PR #873, expected behavior, and references to related PRs/issues. |
Motivation
This PR ensures persistence of the search state in the inbox.
Fixes #782
Type of change:
Please delete options that are not relevant.
Checklist:
Summary by CodeRabbit
New Features
Chores