-
Notifications
You must be signed in to change notification settings - Fork 14
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(SearchTabPanel): Replace ToggleButtonGroup
with Stack
for query option icon buttons
#191
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis update modifies both the styling and state management within the SearchTabPanel component. In the CSS file, explicit dimensions are applied to the query option button using Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchTabPanel
participant QueryService
User ->> SearchTabPanel: Click "Case Sensitivity" or "Regex" button
SearchTabPanel ->> SearchTabPanel: Toggle isCaseSensitive/isRegex state
SearchTabPanel ->> SearchTabPanel: Update IconButton state (aria-pressed)
SearchTabPanel ->> QueryService: Call handleQuerySubmit with updated options
QueryService -->> SearchTabPanel: Return query results
SearchTabPanel -->> User: Render updated search results
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
ToggleButtonGroup
with Stack
for query option icon buttons
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: 0
🧹 Nitpick comments (1)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (1)
68-82
: Consider optimizing the button handlers.The handlers follow a consistent pattern but could be optimized to avoid redundant state updates.
Consider this optimization:
- const handleCaseButtonPressed = () => { - handleQuerySubmit({ - isCaseSensitive: !isCaseSensitive, - isRegex: isRegex, - }); - setIsCaseSensitive(!isCaseSensitive); - }; + const handleCaseButtonPressed = () => { + const newIsCaseSensitive = !isCaseSensitive; + setIsCaseSensitive(newIsCaseSensitive); + handleQuerySubmit({ + isCaseSensitive: newIsCaseSensitive, + isRegex: isRegex, + }); + }; - const handleRegexButtonPressed = () => { - handleQuerySubmit({ - isCaseSensitive: isCaseSensitive, - isRegex: !isRegex, - }); - setIsRegex(!isRegex); - }; + const handleRegexButtonPressed = () => { + const newIsRegex = !isRegex; + setIsRegex(newIsRegex); + handleQuerySubmit({ + isCaseSensitive: isCaseSensitive, + isRegex: newIsRegex, + }); + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css
(1 hunks)src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ...
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
: - Preferfalse == <expression>
rather than!<expression>
.
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx
🧠 Learnings (1)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css (1)
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css:14-18
Timestamp: 2024-11-14T04:35:00.986Z
Learning: In `src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css`, the use of `!important` flags is necessary for overriding styles in the `.query-input-box` class.
🔇 Additional comments (5)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.css (1)
20-24
: LGTM! The button dimensions are appropriate.The explicit dimensions with
!important
flags are necessary and appropriate for icon buttons. Setting both fixed and minimum dimensions ensures consistent button sizing.src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (4)
11-11
: LGTM! The Stack import is appropriate.The Stack component is a suitable replacement for ToggleButtonGroup to manage button layout.
47-48
: LGTM! The state management is simplified.Using boolean flags for case sensitivity and regex options is more straightforward than the previous enum-based approach.
56-58
: LGTM! The query submission is simplified.Direct use of boolean flags makes the code more readable and maintainable by removing the need for utility functions.
111-148
: LGTM! The UI changes are well-implemented.The Stack component with IconButtons provides a clean and accessible replacement for the toggle button group. The buttons are properly configured with tooltips, aria attributes, and consistent styling.
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.
can we create a custom component in src/components
for the toggle buttons? e.g.,
interface ToggleIconButton extends IconButton {
isActive: boolean;
}
Description
ToggleButtonGroup
is causing inconvenience wheneverisCaseSensitive
orisRegex
updates. It is only used because the button pressed status needs to be shown visually. The current implementation achieves the same goal without usingToggleButtonGroup
, and are easier for query options to update.Checklist
breaking change.
Validation performed
Search
Scheduling
without any query options, search results are found and shown;Enable case sensitive, no search results are found.
example.zip