-
Notifications
You must be signed in to change notification settings - Fork 18
feat: search term highlighting within log entry (fixes #189) #305
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughA new Zustand store ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant QueryInputBox
participant useResultsStore
participant Editor
User->>QueryInputBox: Change query input / toggle search options
QueryInputBox->>useResultsStore: setButtonClicked(false)
QueryInputBox->>Editor: Update query state
User->>Result: Click result button
Result->>useResultsStore: setButtonClicked(true)
useResultsStore->>Editor: Notify via subscription
Editor->>Editor: Update find widget with current query params
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (1)`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
🔇 Additional comments (7)
✨ Finishing Touches
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. 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 (
|
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx
Outdated
Show resolved
Hide resolved
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx
Outdated
Show resolved
Hide resolved
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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx
(2 hunks)src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx
(2 hunks)src/components/Editor/index.tsx
(3 hunks)src/stores/resultsStore.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
: - Preferfalse == <expression>
rather than!<expression>
.
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx
src/stores/resultsStore.ts
src/components/Editor/index.tsx
🔇 Additional comments (7)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx (1)
10-10
: LGTM - Clean import addition.The import for the new results store follows the existing patterns in the file.
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx (2)
7-7
: LGTM - Consistent import pattern.The import follows the same pattern established in other components.
41-42
: LGTM - Clear intent and proper ordering.The code correctly sets the button clicked state before updating the URL, ensuring the state change happens first for any subscribers to react to.
src/stores/resultsStore.ts (2)
4-12
: Well-structured interface design.The separation of values and actions into distinct interfaces and their combination into a single state type follows TypeScript best practices and makes the store's API clear.
18-23
: Clean and straightforward Zustand store implementation.The store implementation correctly uses the spread operator for defaults and provides a simple setter function. The implementation is focused and follows Zustand patterns well.
src/components/Editor/index.tsx (2)
1-3
: Verify if ESLint rule relaxation is necessary.The ESLint rules for maximum lines, lines-per-function, and statements were significantly relaxed. Ensure this increase is truly necessary and consider if the code can be refactored to reduce complexity instead.
19-20
: LGTM - Store imports follow established patterns.The new store imports are consistent with the existing codebase patterns.
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx
Show resolved
Hide resolved
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.
overall looks cool
can we change the behaviour a bit to make things more intuitive for the users?
- to make the indices in Monaco's find box accurate, on result button clicks, we can programmatically select the whole log event (before programmatically popping up the find box)
- To keep the matching parts always highlighted, we can run an useEffect hook on any changes of
queryString
,queryIsCaseSensitive
,queryIsRegex
Description
Fixes #189
Search term highlighting enabled within a log event. Syncs the built in monaco editor find actions with the results from the log viewers search.
->find action in monaco editor can take two arguments related to case sensitivity (matchCase and preserveCase). Both arguments take a boolean value. Setting either or both of them doesn't seem to trigger the case sensitivity in the monaco editor.
Checklist
breaking change.
Validation performed
Loaded logs into the log viewer and attempt various searches to see that the correct terms were being highlighted in the log events with a match value.
Summary by CodeRabbit