-
Notifications
You must be signed in to change notification settings - Fork 19
feat: Auto-switch to Search tab when query parameters are present in URL (resolves #331). #364
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: Auto-switch to Search tab when query parameters are present in URL (resolves #331). #364
Conversation
WalkthroughAdds calls to the UI store to set the active sidebar tab to SEARCH when query-related hash parameters are detected or updated, invoked in AppController.handleHashChange and in logFileStore.loadFile before starting a query; also adds an ESLint disable in logFileStore. Changes
Sequence Diagram(s)sequenceDiagram
participant App as AppController
participant Hash as Hash Params
participant UI as UI Store
participant Q as Query Store
App->>Hash: updateViewHashParams()
App->>Hash: updateQueryHashParams()
alt query params updated
App->>UI: setActiveTabName(SEARCH)
App->>Q: startQuery()
else
Note right of App: No query start
end
sequenceDiagram
participant Loader as logFileStore.loadFile
participant Hash as Hash Params
participant UI as UI Store
participant Q as Query Store
Loader->>Hash: updateQueryHashParams()
alt query params updated
Loader->>UI: setActiveTabName(SEARCH)
Loader->>Q: startQuery()
else
Note right of Loader: Continue normal load flow
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the "Integrations" page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (2)📚 Learning: 2025-05-09T01:07:32.803Z
Applied to files:
📚 Learning: 2025-05-27T18:09:40.038Z
Applied to files:
🧬 Code Graph Analysis (1)src/stores/logFileStore.ts (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/components/AppController.tsx
(2 hunks)src/stores/logFileStore.ts
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>
rather than!<expression>
.
Files:
src/components/AppController.tsx
src/stores/logFileStore.ts
🧬 Code Graph Analysis (2)
src/components/AppController.tsx (1)
src/typings/tab.ts (1)
TAB_NAME
(22-22)
src/stores/logFileStore.ts (1)
src/typings/tab.ts (1)
TAB_NAME
(22-22)
🪛 GitHub Check: lint-check
src/stores/logFileStore.ts
[failure] 101-101:
Unused eslint-disable directive (no problems were reported from
🪛 GitHub Actions: lint
src/stores/logFileStore.ts
[error] 101-101: ESLint: Unused eslint-disable directive (no problems were reported from 'max-lines-per-function')
🔇 Additional comments (4)
src/components/AppController.tsx (2)
8-10
: New UI imports for tab switching — LGTMImports of useUiStore and TAB_NAME are appropriate for the new behaviour and keep concerns local. No issues spotted.
34-36
: Auto-switch to Search tab before starting query — aligns with #331Order is correct: setActiveTabName(TAB_NAME.SEARCH) prior to startQuery() ensures users land on the Search tab as the query runs. This matches the feature’s intent.
src/stores/logFileStore.ts (2)
17-17
: TAB_NAME import — LGTMImporting TAB_NAME for consistent tab identifiers is correct and keeps types centralised.
156-160
: Switch to Search tab when query hash params are applied — LGTMSetting the active tab before startQuery() aligns with the UX goal. This mirrors the AppController change and preserves existing query-start flow.
There was a problem hiding this 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/stores/logFileStore.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>
rather than!<expression>
.
Files:
src/stores/logFileStore.ts
🧠 Learnings (2)
📚 Learning: 2025-05-09T01:07:32.803Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Applied to files:
src/stores/logFileStore.ts
📚 Learning: 2025-05-27T18:09:40.038Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#173
File: src/contexts/UrlContextProvider.tsx:1-1
Timestamp: 2025-05-27T18:09:40.038Z
Learning: The UrlContextProvider in src/contexts/UrlContextProvider.tsx is planned to be eventually replaced and deleted, so refactoring efforts to reduce its file size should be avoided despite it being large enough to require eslint-disable max-lines.
Applied to files:
src/stores/logFileStore.ts
🧬 Code Graph Analysis (1)
src/stores/logFileStore.ts (1)
src/typings/tab.ts (1)
TAB_NAME
(22-22)
🔇 Additional comments (2)
src/stores/logFileStore.ts (2)
17-17
: Add TAB_NAME import — required for new tab-switch logicImport is correct and used below for switching the active tab.
154-159
: Auto-switch to Search tab before starting the query — correct placement and behaviourThis aligns with the objective: users land on the Search tab before the query kicks off. Good addition.
if (updateQueryHashParams()) { | ||
const {setActiveTabName} = useUiStore.getState(); | ||
setActiveTabName(TAB_NAME.SEARCH); | ||
const {startQuery} = useQueryStore.getState(); | ||
startQuery(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify no duplicate startQuery/tab-switch triggers across handlers
The AI summary notes AppController.tsx also switches to SEARCH and calls startQuery() when query hash params are present. Ensure we don’t trigger startQuery twice (once here and once in AppController) for the same navigation, which could cause duplicate work.
Consider centralising the “hash-param detected → switch to SEARCH → startQuery” flow in a single place (either AppController or this store) to avoid duplication.
Run this script to locate all call sites:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== updateQueryHashParams call sites ==="
rg -n -A 3 -B 3 $'updateQueryHashParams\\('
echo -e "\n=== startQuery call sites ==="
rg -n -A 3 -B 3 $'startQuery\\('
echo -e "\n=== setActiveTabName call sites ==="
rg -n -A 3 -B 3 $'setActiveTabName\\('
Length of output: 9733
Centralize hash-param handling to avoid duplicate searches
Both logFileStore.ts
and AppController.tsx
detect updateQueryHashParams()
, switch to the SEARCH tab, and call startQuery()
. This duplication can fire two identical queries on load.
Please consolidate this flow in one place or add a guard to prevent back-to-back triggers:
• src/stores/logFileStore.ts (lines 154–159)
• src/components/AppController.tsx (lines 33–38)
Consider moving the hash-param → tab switch → startQuery()
logic entirely into AppController
, or vice versa, and remove the redundant block.
🤖 Prompt for AI Agents
In src/stores/logFileStore.ts around lines 154–159, the code duplicates the
hash-param handling already implemented in src/components/AppController.tsx
(lines 33–38), causing two identical queries on load; remove or disable the
block in logFileStore.ts and centralize the hash-param →
setActiveTabName(TAB_NAME.SEARCH) → startQuery() flow in a single location
(preferably AppController), or add a guard (e.g., a boolean flag in a shared
store or check if the SEARCH tab is already active or query already started) to
prevent back-to-back triggers so only one startQuery() executes when
updateQueryHashParams() is true.
🧹 Nitpick (assertive)
Guard against redundant tab updates to avoid unnecessary re-renders
If the UI is already on the Search tab, skip dispatching setActiveTabName to reduce churn.
Apply this diff:
- const {setActiveTabName} = useUiStore.getState();
- setActiveTabName(TAB_NAME.SEARCH);
+ const {activeTabName, setActiveTabName} = useUiStore.getState();
+ if (TAB_NAME.SEARCH !== activeTabName) {
+ setActiveTabName(TAB_NAME.SEARCH);
+ }
📝 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.
if (updateQueryHashParams()) { | |
const {setActiveTabName} = useUiStore.getState(); | |
setActiveTabName(TAB_NAME.SEARCH); | |
const {startQuery} = useQueryStore.getState(); | |
startQuery(); | |
} | |
if (updateQueryHashParams()) { | |
const {activeTabName, setActiveTabName} = useUiStore.getState(); | |
if (TAB_NAME.SEARCH !== activeTabName) { | |
setActiveTabName(TAB_NAME.SEARCH); | |
} | |
const {startQuery} = useQueryStore.getState(); | |
startQuery(); | |
} |
🤖 Prompt for AI Agents
In src/stores/logFileStore.ts around lines 154 to 159, guard against redundant
tab updates by reading the current active tab from useUiStore.getState() (e.g.,
const { activeTabName, setActiveTabName } = useUiStore.getState()), compare
activeTabName to TAB_NAME.SEARCH, and only call
setActiveTabName(TAB_NAME.SEARCH) when they differ; keep calling startQuery() as
before if updateQueryHashParams() returns true. Ensure you use the store's
getState() accessor without introducing side effects and preserve existing
typing.
@quinntaylormitchell, can you take a shot at reviewing this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…upon-query # Conflicts: # src/components/AppController.tsx
Description
Resolves #331.
Checklist
breaking change.
Validation performed
queryIsCaseSensitive=true
andqueryIsRegex=true
, side bar also switches.Summary by CodeRabbit