feat: add search online function#23
Conversation
WalkthroughUpdates dependency for Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend as OnlineSearchPage / HomeOptions
participant PluginOpener as @tauri-apps/plugin-opener
participant TauriAPI as Tauri JS API
participant Backend as src-tauri/lib.rs
User->>Frontend: Type query and select / press Enter
Frontend->>Frontend: handleSearch(query) (validate, build Google URL)
Frontend->>PluginOpener: openUrl(googleSearchURL)
PluginOpener->>PluginOpener: Launch system browser
Frontend->>TauriAPI: invoke('hide_window')
TauriAPI->>Backend: hide_window command
Backend->>Backend: hide/toggle main window
Backend-->>TauriAPI: ack
TauriAPI-->>Frontend: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (3)
package.json(1 hunks)src-tauri/src/lib.rs(5 hunks)src/components/OnlineSearchPage.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/OnlineSearchPage.jsx (1)
src/hooks/useKeyboardNavigation.js (1)
useKeyboardNavigation(3-61)
🪛 Biome (2.1.2)
src/components/OnlineSearchPage.jsx
[error] 34-38: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 34-38: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
🔇 Additional comments (5)
src-tauri/src/lib.rs (3)
499-504: Return a Result from the Tauri command.Expose errors to the frontend and keep commands consistent with others returning Result.
[raise_recommended_refactor]-#[tauri::command] -fn hide_window(app: tauri::AppHandle) { - if let Some(window) = app.get_webview_window("main") { - let _ = window.hide(); - } -} +#[tauri::command] +fn hide_window(app: tauri::AppHandle) -> Result<(), String> { + let win = app + .get_webview_window("main") + .ok_or_else(|| "window 'main' not found".to_string())?; + win.hide().map_err(|e| e.to_string()) +}
574-574: Opener plugin initialization added correctly.No concerns here. Ensure corresponding permission scopes exist; otherwise openUrl will be blocked. (v2.tauri.app)
577-578: Global shortcut registration is correct for v2.Builder handler + separate register() call is the right pattern in v2.
Confirm capabilities for global-shortcut are enabled; v2 plugins are permission‑gated.
Also applies to: 604-611
src/components/OnlineSearchPage.jsx (1)
2-4: Correct API import for opener; confirm permissions.openUrl import is correct for plugin‑opener v2. Ensure opener URL scope is allowed in capabilities; otherwise this will reject at runtime. (v2.tauri.app)
package.json (1)
18-18: All plugin version and capabilities concerns verified—no issues found.Verification confirms:
- Rust
tauri-plugin-opener = "2"aligns with JS^2.5.0(both v2.x compatible)tauri-plugin-global-shortcut = "2.3.0"is compatible- Capabilities include
"opener:default"permission, which grantsopenUrlscope by default- Configuration is complete and ready
| { | ||
| let path = entry.path(); | ||
| if is_app_file(&path.to_path_buf()) { | ||
| let path = entry.path(); if is_app_file(&path.to_path_buf()) { |
There was a problem hiding this comment.
Formatting glitch and missed AppImage match.
Split statements for readability; also ensure AppImage is detected (is_app_file currently matches "AppImage" but extensions are lowercased).
- let path = entry.path(); if is_app_file(&path.to_path_buf()) {
+ let path = entry.path();
+ if is_app_file(&path.to_path_buf()) {Additionally (outside this range), fix extension match:
- "deb" | "rpm" | "AppImage" => true, // Linux
+ "deb" | "rpm" | "appimage" => true, // Linux📝 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.
| let path = entry.path(); if is_app_file(&path.to_path_buf()) { | |
| let path = entry.path(); | |
| if is_app_file(&path.to_path_buf()) { |
🤖 Prompt for AI Agents
In src-tauri/src/lib.rs around line 245, split the combined let/path check into
two statements for readability (first let path = entry.path(); then an if using
path), and update is_app_file so AppImage detection is case-insensitive by
lowercasing the extension or filename before matching (currently it matches
"AppImage" literally); also adjust the other extension-match locations (outside
this range) to compare lowercase extensions or names so matches like ".AppImage"
or "AppImage" in different cases are detected consistently.
| // --- FIX 1: Define the handler logic --- | ||
| // This handler will be attached to the main builder. | ||
| // It must be able to check *which* shortcut was pressed. | ||
| let shortcut_handler = ShortcutBuilder::new() | ||
| .with_handler(move |app, scut, event| { | ||
| // Re-create the shortcut struct to compare its ID | ||
| let shortcut = Shortcut::new(Some(Modifiers::CONTROL | Modifiers::SHIFT), Code::Space); | ||
|
|
||
| if scut.id() == shortcut.id() && event.state() == ShortcutState::Pressed { | ||
| let win = app.get_webview_window("main").expect("window not found"); | ||
| if win.is_visible().unwrap_or(false) { | ||
| let _ = win.hide(); | ||
| } else { | ||
| let _ = win.show(); | ||
| let _ = win.set_focus(); | ||
| } | ||
| } | ||
| }) | ||
| .build(); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Shortcut handler logic is sound; minor hardening optional.
Comparison via id + Pressed state is correct. Consider logging errors from show()/hide()/set_focus() for diagnosability.
🤖 Prompt for AI Agents
In src-tauri/src/lib.rs around lines 553 to 572, the shortcut handler currently
ignores results from window operations (show/hide/set_focus); update the handler
to check the Result returned by each call and log any errors for diagnosability.
Specifically, after calling win.hide(), win.show(), and win.set_focus(), match
or use if let Err(e) to capture errors and forward them to your logging facility
(e.g., processLogger/tracing or the crate logger) with a clear message including
the operation and the error; keep the existing visibility toggle logic intact.
| @@ -1,19 +1,44 @@ | |||
| import { useEffect } from "react"; | |||
There was a problem hiding this comment.
Remove unused import.
useEffect isn’t used anymore.
-import { useEffect } from "react";
+// (unused) import removedCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/OnlineSearchPage.jsx around lines 1 to 1, the import statement
includes useEffect which is no longer used; remove useEffect from the import (or
delete the entire import line if nothing else is imported) to eliminate the
unused import and satisfy the linter.
| const { getItemProps } = useKeyboardNavigation([query], async (item, idx) => { | ||
| await handleSearch(query); | ||
| }); |
There was a problem hiding this comment.
Avoid stale closure: use the item passed by the hook.
Use item to ensure the searched query matches the selected entry.
- const { getItemProps } = useKeyboardNavigation([query], async (item, idx) => {
- await handleSearch(query);
- });
+ const { getItemProps } = useKeyboardNavigation([query], async (item) => {
+ await handleSearch(item);
+ });📝 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.
| const { getItemProps } = useKeyboardNavigation([query], async (item, idx) => { | |
| await handleSearch(query); | |
| }); | |
| const { getItemProps } = useKeyboardNavigation([query], async (item) => { | |
| await handleSearch(item); | |
| }); |
🤖 Prompt for AI Agents
In src/components/OnlineSearchPage.jsx around lines 8 to 10, the keyboard
navigation callback is closing over the external query variable instead of using
the item passed by the hook; update the callback to call handleSearch with the
item provided by useKeyboardNavigation (e.g., handleSearch(item) or
handleSearch(item.value/label depending on the item shape), and validate the
item exists and is a string before invoking handleSearch to avoid stale closures
and incorrect searches.
| const handleSearch = async (searchQuery) => { | ||
| if (!searchQuery.trim()) return; | ||
|
|
||
| try { | ||
| // Construct search URL (Google search) | ||
| const searchUrl = `https://www.google.com/search?q=${encodeURIComponent(searchQuery)}`; | ||
|
|
||
| // 3. Call the correct 'openUrl' function | ||
| await openUrl(searchUrl); | ||
|
|
||
| // Hide the PathFinder window | ||
| await invoke('hide_window'); | ||
| } catch (error) { | ||
| console.error("Failed to open browser:", error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Handle permission errors and keep UX responsive.
If openUrl is denied by capabilities, this throws. Consider surfacing a toast and keeping the window visible on failure. Also verify opener scopes include https URLs. Docs detail URL scope configuration. (v2.tauri.app)
🤖 Prompt for AI Agents
In src/components/OnlineSearchPage.jsx around lines 15 to 30, the try/catch
around openUrl doesn't handle permission-denied failures or keep the UI
responsive: update the catch to detect capability/permission errors (e.g., check
error message/code), show a user-facing toast notification describing the
failure, and do NOT call invoke('hide_window') on failure so the PathFinder
window remains visible; additionally, before calling openUrl verify the opener
scope supports https URLs (ensure configuration or gate calls by testing the URL
scheme) and if scope is insufficient surface a clear toast telling the
user/guide them to update opener scopes.
| <div | ||
| {...getItemProps(0)} | ||
| className={`option-item ${getItemProps(0).className}`} | ||
| onClick={() => handleSearch(query)} | ||
| > | ||
| <span className="icon">🌐</span> | ||
| <span>Search online for “{query}”</span> | ||
| <span>Search online for "{query}"</span> | ||
| </div> |
There was a problem hiding this comment.
Drop duplicate className derivation.
getItemProps already includes className; avoid concatenating it again.
Included in the button refactor above.
🧰 Tools
🪛 Biome (2.1.2)
[error] 34-38: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 34-38: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
🤖 Prompt for AI Agents
In src/components/OnlineSearchPage.jsx around lines 34 to 41, the div spreads
getItemProps(0) and then appends getItemProps(0).className again which
duplicates and calls getItemProps twice; remove the concatenation and rely on
the className provided by getItemProps (or call getItemProps once into a const
and use its className if you need to augment it), so change the element to use
the single getItemProps result for className without re-appending its className.
Fix a11y: use a button, not an interactive div.
Addresses Biome errors: static element interactions and missing keyboard equivalence.
- <div
- {...getItemProps(0)}
- className={`option-item ${getItemProps(0).className}`}
- onClick={() => handleSearch(query)}
- >
+ <button
+ {...getItemProps(0)}
+ type="button"
+ aria-label={`Search online for ${query}`}
+ >
<span className="icon">🌐</span>
<span>Search online for "{query}"</span>
- </div>
+ </button>Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (2.1.2)
[error] 34-38: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 34-38: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
🤖 Prompt for AI Agents
In src/components/OnlineSearchPage.jsx around lines 34 to 41, the interactive
div used for the "Search online for {query}" item is not accessible; replace it
with a semantic <button type="button"> element (preserve/getItemProps values on
the button, merge className into className prop), remove any manual
role/keyboard handlers, keep the onClick calling handleSearch(query), and add a
concise aria-label if needed (e.g., `aria-label={`Search online for ${query}`}`)
so the control is keyboard-focusable and properly announced.
|
Hey @Larry8668 Done with the issue you can review it |
|
Hey @TanishqDNEC , sorry for the late review the search is working when i am inside the How to replicate this:
In the current state of app, when you open it - typing anything that is not is the 3 section will show Search online with ... See if you can get this to work - otherwise the PR looks good |
|
I will create a new PR for the web search + summary in-app I am thinking of what more can be added to this app - do let me know if you have any cool ideas |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/components/HomeOptions.jsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/HomeOptions.jsx (2)
src/components/OnlineSearchPage.jsx (2)
handleSearch(15-30)useKeyboardNavigation(8-10)src/hooks/useKeyboardNavigation.js (1)
useKeyboardNavigation(3-61)
🔇 Additional comments (3)
src/components/HomeOptions.jsx (3)
2-3: LGTM! Required imports for web search functionality.The imports are correctly added to support opening URLs in the browser and invoking Tauri commands.
40-42: Excellent! This addresses the maintainer's feedback.The fallback web search option when no local matches are found directly addresses the maintainer's concern about making search work globally (not just within the Online Search tab). Users can now type any query and get a web search option even when it doesn't match local options.
44-53: LGTM! Clean branching logic for navigation.The keyboard navigation callback correctly handles both local navigation and web search flows. The distinction via
isWebSearchflag is clear, and both paths properly clean up (web search clears viahandleSearch, regular navigation clears explicitly).
| const handleSearch = async (searchQuery) => { | ||
| if (!searchQuery.trim()) return; | ||
|
|
||
| try { | ||
| // Construct search URL (Google search) | ||
| const searchUrl = `https://www.google.com/search?q=${encodeURIComponent(searchQuery)}`; | ||
|
|
||
| // Open URL in browser | ||
| await openUrl(searchUrl); | ||
|
|
||
| // Hide the PathFinder window | ||
| await invoke('hide_window'); | ||
|
|
||
| // Clear query | ||
| clearQuery(); | ||
| } catch (error) { | ||
| console.error("Failed to open browser:", error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting the duplicate handleSearch logic.
The implementation is correct and handles the search flow properly (URL construction, opening browser, hiding window, clearing query). However, this logic is nearly identical to the handleSearch function in OnlineSearchPage.jsx (lines 14-29).
Consider extracting this shared logic into a custom hook or utility function:
// src/utils/webSearch.js or src/hooks/useWebSearch.js
export async function performWebSearch(searchQuery, clearQuery) {
if (!searchQuery.trim()) return;
try {
const searchUrl = `https://www.google.com/search?q=${encodeURIComponent(searchQuery)}`;
await openUrl(searchUrl);
await invoke('hide_window');
clearQuery();
} catch (error) {
console.error("Failed to open browser:", error);
}
}Then use it in both components:
const handleSearch = async (searchQuery) => {
await performWebSearch(searchQuery, clearQuery);
};🤖 Prompt for AI Agents
In src/components/HomeOptions.jsx around lines 19 to 37, the handleSearch
implementation duplicates logic found in OnlineSearchPage.jsx; extract the
shared behavior into a single exported utility or hook (e.g.,
src/utils/webSearch.js or src/hooks/useWebSearch.js) that accepts the
searchQuery and a clearQuery callback, moves URL construction, openUrl/invoke
calls and error handling into that function, and then replace the local
handleSearch with a simple call to the new performWebSearch/useWebSearch so both
HomeOptions.jsx and OnlineSearchPage.jsx import and reuse the same
implementation.
| <div | ||
| {...getItemProps(idx)} | ||
| className={`option-item ${getItemProps(idx).className}`} | ||
| key={idx} |
There was a problem hiding this comment.
Fix className duplication and inefficient getItemProps calls.
There are two issues here:
-
Duplicate className: Spreading
{...getItemProps(idx)}applies the className from the hook (which already includes "option-item"), then explicitly settingclassName={...}overrides it with a duplicated "option-item" class. SincegetItemPropsreturnsclassName: "option-item selected"(or just"option-item"), the result isclassName="option-item option-item selected". -
Inefficient calls:
getItemProps(idx)is called twice per render (lines 59 and 60), which is wasteful.
Apply this diff to fix both issues:
- {itemsToShow.map((opt, idx) => (
- <div
- {...getItemProps(idx)}
- className={`option-item ${getItemProps(idx).className}`}
- key={idx}
- >
+ {itemsToShow.map((opt, idx) => {
+ const itemProps = getItemProps(idx);
+ return (
+ <div
+ {...itemProps}
+ key={idx}
+ >
<span className="icon">{opt.icon}</span>
<span>{opt.title}</span>
</div>
- ))}
+ )})}The getItemProps hook already returns the complete className with both "option-item" and the "selected" state, so the spread operator alone is sufficient.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/HomeOptions.jsx around lines 58 to 61, getItemProps(idx) is
called twice and its returned className is being overridden/duplicated by an
explicit className prop; call getItemProps(idx) once, store its result in a
local variable (e.g., const itemProps = getItemProps(idx)), spread only that
object into the div (remove the explicit className override) and keep the key
prop as before so the returned className (which already includes "option-item"
and "selected" when appropriate) is used without duplication and without
redundant calls.
|
Hey @Larry8668 I have fixed the bug now typing random query will open the default browser with the query |
|
Regarading the summary feature ..Right now you can work on that |
2 participants
closes issue #8
Feature: Web Search Implementation
This document outlines the implementation of the "Search Online" feature.
1. Frontend (
OnlineSearchPage.jsx)The React component
OnlineSearchPage.jsxprovides the UI and frontend logic.onClick) or by the "Enter" key, managed by theuseKeyboardNavigationhook.handleSearchfunction constructs a Google search URL and uses theopenUrlmethod (fromtauri-plugin-opener) to open the URL in the user's default browser.hide_windowTauri command to conceal the application window.2. Backend (
main.rs)The Rust backend supplies the native functionality required by the frontend.
hide_windowCommand: A#[tauri::command]function namedhide_windowis implemented to find the "main" window and call its.hide()method.tauri-plugin-openeris initialized in thetauri::Builder. Thehide_windowcommand is registered via theinvoke_handlerto make it accessible from the frontend.3. Dependencies
The implementation relies on the following key dependencies:
tauri-plugin-opener: Provides the coreopenUrlfunctionality.Summary by CodeRabbit