-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Log viewer: Improves search functionality and code quality #20913
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
Conversation
…ture - Replace RxJS Subject with UmbStringState to follow Umbraco patterns - Move debounced search observation to messages list component - Only triggers when component is mounted (logs are visible) - Prevents unnecessary API calls on other views - Simplify search input to just update context state - Add semantic form structure with role="search" for accessibility - Add visually-hidden submit button for keyboard navigation - Allow re-running same query via form submission (bypasses debounce) - Follow same architecture pattern as date range selector This resolves the TODO to not use RxJS directly and significantly improves separation of concerns where the data consumer (messages list) owns the fetching logic. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add refresh button with icon-refresh next to save and clear buttons - Allows users to re-run search with same query (bypasses debounce) - Remove form structure that couldn't work due to Shadow DOM boundaries - Simplify parent component by removing form submission logic - Keep role="search" for accessibility The refresh button provides a more discoverable UI than the hidden submit button approach and avoids Shadow DOM event bubbling issues. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add local UmbStringState to debounce user input (250ms) - Only update context filterExpression after debounce - Remove debouncing from messages list (now handled at input level) - Saved searches and refresh button still bypass debounce for immediate feedback This restores the expected debouncing behavior while maintaining the clean architecture where the messages list triggers searches based on context changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
Pull Request Overview
This PR refactors the log viewer search functionality to improve code quality and user experience. The main achievement is replacing RxJS Subject with the Umbraco-standard UmbStringState pattern, improving architecture by moving search triggering responsibility to the data consumer (messages list), and adding a user-requested refresh button.
Key changes:
- Replaces RxJS Subject with UmbStringState for better alignment with Umbraco patterns
- Moves search triggering logic from input component to messages list component (data consumer owns data fetching)
- Adds refresh button functionality to re-run searches without toggling polling
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| log-viewer-search-input.element.ts | Refactors search input to use UmbStringState for debouncing, removes loader UI, adds refresh button functionality |
| log-viewer-messages-list.element.ts | Adds observer for filterExpression changes to trigger search when component is mounted and filter changes |
| log-viewer-date-range-selector.element.ts | Adds nullish coalescing to prevent console errors when dateRange is undefined |
| log-search-view.element.ts | Adds accessibility attributes (role and aria-label) to search header |
...src/packages/log-viewer/workspace/views/search/components/log-viewer-search-input.element.ts
Show resolved
Hide resolved
...src/packages/log-viewer/workspace/views/search/components/log-viewer-search-input.element.ts
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
leekelleher
left a comment
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.
Tested out, all working as described. 🚀
Summary
Refactors the log viewer search functionality to improve code quality and add user-requested features.
Changes:
Benefits:
Test Steps
Basic Search Functionality
Refresh Button
Saved Searches
URL Parameters
?lq=Error&startDate=2025-11-19&endDate=2025-11-20Date Range Handling