This commit aims to solve issue (#6 Add an intro to the app on first … - #21
This commit aims to solve issue (#6 Add an intro to the app on first …#21AlokPy1484 wants to merge 3 commits into
Conversation
…time launch #10) * Added a welcome page * Added a guide on how to use all the features * Added a form to save user input name to localStorage.
WalkthroughThis PR introduces a React Router-based multi-page navigation structure with a first-launch onboarding flow, adds seven guide pages covering app features, integrates Tailwind CSS for styling, removes file search functionality from the Tauri backend, and updates dependencies accordingly. Changes
Sequence DiagramsequenceDiagram
participant User
participant App as App.jsx
participant Hook as useFirstLaunch()
participant Storage as localStorage
participant Router as React Router
User->>App: Open app
App->>Hook: Call useFirstLaunch()
Hook->>Storage: Check "firstLaunch" key
alt First Launch
Storage-->>Hook: Key not found
Hook->>Storage: Set "firstLaunch" = "true"
Hook-->>App: Return true
App->>Router: Render <Route path="/" element={WelcomePage} />
Router-->>User: Show Welcome Page
Note over User: User enters name → About → Guides → GuideEnd
else Subsequent Launch
Storage-->>Hook: Key exists
Hook-->>App: Return false
App->>Router: Render <Route path="/" element={HomePage} />
Router-->>User: Show Home (Search) Page
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes This PR involves substantial structural refactoring: a complete navigation architecture migration, removal of backend file search subsystem, eight new page components (though following similar patterns), dependency additions, and CSS refactoring. The multi-file scope and mix of frontend UI changes, backend removal, and configuration updates require careful verification across integration points, despite individual components being relatively straightforward. Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/App.jsx (1)
20-33: Stale router‑agnostic Escape handling and unused state.
currentPage,query, andinputRefaren’t integrated with routes; Escape logic won’t navigate correctly. Use routeruseLocation/useNavigateinstead.-import { BrowserRouter, Routes, Route } from "react-router-dom"; +import { BrowserRouter, Routes, Route, useLocation, useNavigate } from "react-router-dom"; @@ - const [query, setQuery] = useState(""); - const inputRef = useRef(null); - const [currentPage, setCurrentPage] = useState("home"); + const navigate = useNavigate(); + const location = useLocation(); @@ - useEffect(() => { - function handleKeyDown(e) { - if (e.key === "Escape") { - if (currentPage === "home") { - getCurrentWindow().hide(); - } else { - setCurrentPage("home"); - setQuery(""); - inputRef.current?.focus(); - } - } - } - - window.addEventListener("keydown", handleKeyDown); - return () => window.removeEventListener("keydown", handleKeyDown); - }, [currentPage]); + useEffect(() => { + function handleKeyDown(e) { + if (e.key !== "Escape") return; + if (location.pathname === "/" || location.pathname === "/home") { + getCurrentWindow().hide(); + } else { + navigate("/home", { replace: true }); + } + } + window.addEventListener("keydown", handleKeyDown); + return () => window.removeEventListener("keydown", handleKeyDown); + }, [location.pathname, navigate]); @@ - useEffect(() => { - inputRef.current?.focus(); - }, []); + // If focusing a search input is still desired, handle it within that page component.Note: Wrap Routes with a component that has access to hooks, e.g., move this logic into a
RouterShellthat renders the<Routes>.Also applies to: 36-39
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
src/assets/snapshort2.pngis excluded by!**/*.pngsrc/assets/snapshort3.pngis excluded by!**/*.pngsrc/assets/snapshort4.pngis excluded by!**/*.pngsrc/assets/snapshot1.pngis excluded by!**/*.png
📒 Files selected for processing (13)
src/App.css(1 hunks)src/App.jsx(2 hunks)src/components/HomeOptions.jsx(1 hunks)src/components/OpenFilePage.jsx(1 hunks)src/hooks/useFirstLaunch.jsx(1 hunks)src/pages/About.jsx(1 hunks)src/pages/ClipboardGuide.jsx(1 hunks)src/pages/GuideEnd.jsx(1 hunks)src/pages/HomePage.jsx(1 hunks)src/pages/Name.jsx(1 hunks)src/pages/OnlineSearchGuide.jsx(1 hunks)src/pages/OpenFileGuide.jsx(1 hunks)src/pages/WelcomePage.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/pages/HomePage.jsx (5)
src/hooks/useKeyboardNavigation.js (1)
handleKeyDown(24-45)src/components/HomeOptions.jsx (1)
HomeOptions(12-42)src/components/ClipboardPage.jsx (1)
ClipboardPage(8-200)src/components/OnlineSearchPage.jsx (1)
OnlineSearchPage(3-19)src/components/OpenFilePage.jsx (1)
OpenFilePage(12-35)
src/App.jsx (2)
src/hooks/useFirstLaunch.jsx (2)
isFirstLaunch(4-4)useFirstLaunch(3-18)src/pages/HomePage.jsx (3)
query(11-11)inputRef(12-12)currentPage(13-13)
src/components/OpenFilePage.jsx (6)
src/components/HomeOptions.jsx (2)
fuse(10-10)useKeyboardNavigation(17-20)src/components/ClipboardPage.jsx (2)
fuse(113-116)useKeyboardNavigation(122-122)src/App.jsx (1)
query(20-20)src/pages/HomePage.jsx (1)
query(11-11)src/hooks/useKeyboardNavigation.js (1)
useKeyboardNavigation(3-61)src/components/OnlineSearchPage.jsx (1)
useKeyboardNavigation(4-6)
src/hooks/useFirstLaunch.jsx (1)
src/App.jsx (1)
isFirstLaunch(16-16)
🪛 Biome (2.1.2)
src/pages/WelcomePage.jsx
[error] 15-15: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
src/pages/About.jsx
[error] 20-20: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
src/pages/OpenFileGuide.jsx
[error] 16-16: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
src/pages/ClipboardGuide.jsx
[error] 25-25: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
src/pages/OnlineSearchGuide.jsx
[error] 15-15: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
src/pages/GuideEnd.jsx
[error] 11-11: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
src/pages/Name.jsx
[error] 32-32: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (7)
src/components/HomeOptions.jsx (1)
7-7: LGTM! Page value updated to align with new routing.The rename from "open-app" to "open-file" correctly reflects the new OpenFileGuide route structure introduced in this PR.
src/App.css (2)
1-1: LGTM! Tailwind CSS integration added.The global Tailwind import enables utility-first styling across the new page components.
3-265: File search styles removed to align with simplified OpenFilePage.The removal of dedicated file-search CSS aligns with the transition to a router-based structure and the simplified OpenFilePage implementation using dummy data.
src/pages/HomePage.jsx (1)
9-69: LGTM! HomePage implements clean sub-page navigation with proper focus management.The component correctly:
- Manages local state for query and current page
- Handles Escape key for navigation (hide window on home, reset to home otherwise)
- Auto-focuses the search input on mount
- Conditionally renders sub-pages based on currentPage state
The implementation aligns well with the simplified page architecture introduced in this PR.
src/App.jsx (1)
54-55: Route path casing.React Router matches paths case‑sensitively by default. Ensure links consistently use “/About” (capital A) everywhere.
src/pages/About.jsx (1)
19-21: Shortcut text might differ by OS.Consider showing platform‑specific shortcut (e.g., Cmd on macOS).
src/pages/ClipboardGuide.jsx (1)
2-3: The review comment is incorrect—the imports will not fail at build time.The actual files in
src/assets/aresnapshot1.pngandsnapshort2.png. The import paths in lines 2–3 correctly reference these existing files:
- Line 2:
'../assets/snapshot1.png'✓ (file exists)- Line 3:
'../assets/snapshort2.png'✓ (file exists, notsnapshot2.png)The review claimed
snapshort2.pngshould besnapshot2.png, but that file does not exist in the assets directory. The import will resolve successfully as written.While there is minor inconsistency in the variable naming (e.g.,
snapshort1vs.snapshot1prefix), this is not a breaking issue—only a stylistic one.Likely an incorrect or invalid review comment.
| const isFirstLaunch = useFirstLaunch(); | ||
| console.log("isFirstLaunch:", isFirstLaunch); | ||
|
|
There was a problem hiding this comment.
Guard initial null state and drop debug log.
Avoid a flash of Home when isFirstLaunch === null; remove console.log.
- const isFirstLaunch = useFirstLaunch();
- console.log("isFirstLaunch:", isFirstLaunch);
+ const isFirstLaunch = useFirstLaunch();
+
+ if (isFirstLaunch === null) {
+ return null; // or a splash/loading component
+ }Also applies to: 45-61
🤖 Prompt for AI Agents
In src/App.jsx around lines 16-18 (and similarly for the render logic at lines
45-61), remove the debug console.log and add a guard for the initial null state
of isFirstLaunch so the component returns a neutral/loading placeholder (or
null) while isFirstLaunch === null to avoid flashing the Home screen; then
continue rendering the normal UI only when isFirstLaunch is true/false.
| {filteredFiles.map((file, idx) => ( | ||
| <div | ||
| {...getItemProps(idx)} | ||
| className={`option-item ${getItemProps(idx).className}`} | ||
| key={idx} |
There was a problem hiding this comment.
Don’t override spread props; avoid double getItemProps call; use stable keys.
{...getItemProps(idx)} sets className which you override immediately, duplicating classes. Also prefer a stable key.
- {filteredFiles.map((file, idx) => (
- <div
- {...getItemProps(idx)}
- className={`option-item ${getItemProps(idx).className}`}
- key={idx}
- >
+ {filteredFiles.map((file, idx) => {
+ const itemProps = getItemProps(idx);
+ return (
+ <div
+ {...itemProps}
+ className={`option-item ${itemProps.className ?? ""}`}
+ key={file.text}
+ >
<span className="icon">📁</span>
<span>{file.text}</span>
- </div>
- ))}
+ </div>
+ );
+ })}📝 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.
| {filteredFiles.map((file, idx) => ( | |
| <div | |
| {...getItemProps(idx)} | |
| className={`option-item ${getItemProps(idx).className}`} | |
| key={idx} | |
| {filteredFiles.map((file, idx) => { | |
| const itemProps = getItemProps(idx); | |
| return ( | |
| <div | |
| {...itemProps} | |
| className={`option-item ${itemProps.className ?? ""}`} | |
| key={file.text} | |
| > | |
| <span className="icon">📁</span> | |
| <span>{file.text}</span> | |
| </div> | |
| ); | |
| })} |
🤖 Prompt for AI Agents
In src/components/OpenFilePage.jsx around lines 23 to 27, avoid calling
getItemProps twice and overriding its className: call getItemProps once (e.g.,
const itemProps = getItemProps(idx)), merge className by combining
itemProps.className with your own additional classes instead of overwriting,
spread the resulting merged props onto the div, and use a stable key (for
example file.id or file.path) instead of the array index to prevent React key
instability.
| export function useFirstLaunch() { | ||
| const [isFirstLaunch, setIsFirstLaunch] = useState(null); | ||
|
|
||
| useEffect(() => { | ||
| const flag = localStorage.getItem("firstLaunch"); | ||
|
|
||
| if (flag === null) { | ||
| localStorage.setItem("firstLaunch", "true"); | ||
| setIsFirstLaunch(true); | ||
| } else { | ||
| setIsFirstLaunch(false); | ||
| } | ||
| }, []); | ||
|
|
||
| return isFirstLaunch; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
First-launch flag is never cleared after initial detection.
The hook sets localStorage.setItem("firstLaunch", "true") on first launch but never removes it. This means after the first launch completes, isFirstLaunch will always return false on subsequent app opens, which is correct. However, there's no mechanism to clear this flag if the user wants to see the intro again (e.g., via settings).
According to the PR objectives, a settings feature will be added later to manage this behavior. Consider adding a helper function or documenting how to reset the first-launch state for future integration.
Do you want me to help design a reset mechanism or document the expected behavior for the future settings integration?
| <img src={snapshort4} alt='snapshot1' | ||
| className="flex justify-center items-center rounded-t-xl "/> | ||
| </div> |
There was a problem hiding this comment.
Improve image accessibility and remove odd “flex” on img.
Provide descriptive alt text; flex on <img> is unnecessary.
- <img src={snapshort4} alt='snapshot1'
- className="flex justify-center items-center rounded-t-xl "/>
+ <img
+ src={snapshort4}
+ alt="Screenshot: Open File feature showing filtered results"
+ className="rounded-t-xl"
+ />📝 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.
| <img src={snapshort4} alt='snapshot1' | |
| className="flex justify-center items-center rounded-t-xl "/> | |
| </div> | |
| <img | |
| src={snapshort4} | |
| alt="Screenshot: Open File feature showing filtered results" | |
| className="rounded-t-xl" | |
| /> | |
| </div> |
🤖 Prompt for AI Agents
In src/pages/OpenFileGuide.jsx around lines 13 to 15, the <img> element uses a
non-descriptive alt ('snapshot1') and includes an unnecessary 'flex' utility in
its className; update the alt to a meaningful description of the image content
(e.g., what the snapshot shows) and remove 'flex' from the className (replace
with appropriate image layout utilities such as block or mx-auto and responsive
sizing if needed) so the image is accessible and not mis-styled.
| <img src={snapshort4} alt='snapshot1' | ||
| className="flex justify-center items-center rounded-t-xl "/> | ||
| </div> | ||
| <Link to='/GuideEnd'><button className="relative bottom-3 left-170 flex flex-row justify-center py-1 px-3 bg-black rounded-md text-center text-white ">next</button></Link> |
There was a problem hiding this comment.
Explicit button type + avoid invalid Tailwind class.
Add type="button" (a11y). left-170 isn’t a standard utility; prefer layout utilities.
- <Link to='/GuideEnd'><button className="relative bottom-3 left-170 flex flex-row justify-center py-1 px-3 bg-black rounded-md text-center text-white ">next</button></Link>
+ <Link to="/GuideEnd">
+ <button
+ type="button"
+ className="self-end py-1 px-3 bg-black rounded-md text-white"
+ >
+ Next
+ </button>
+ </Link>📝 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.
| <Link to='/GuideEnd'><button className="relative bottom-3 left-170 flex flex-row justify-center py-1 px-3 bg-black rounded-md text-center text-white ">next</button></Link> | |
| <Link to="/GuideEnd"> | |
| <button | |
| type="button" | |
| className="self-end py-1 px-3 bg-black rounded-md text-white" | |
| > | |
| Next | |
| </button> | |
| </Link> |
🧰 Tools
🪛 Biome (2.1.2)
[error] 16-16: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🤖 Prompt for AI Agents
src/pages/OpenFileGuide.jsx around line 16: the button lacks an explicit type
attribute and uses a nonstandard Tailwind class "left-170". Add type="button" to
the button element and replace the invalid utility with a valid Tailwind layout
utility — either a semantic margin (e.g., ml-40 or another appropriate ml-
value) or an arbitrary value syntax (e.g., left-[170px]) if absolute positioning
is required; ensure the chosen utility matches the surrounding layout (keep
relative positioning if using left-[170px] or switch to margin for flow-based
spacing).
|
|
||
|
|
||
| function Welcome(){ | ||
| console.log(localStorage.getItem("profile")) |
There was a problem hiding this comment.
Remove debug console.log statement.
This console.log appears to be leftover debug code. Additionally, it references a "profile" key in localStorage, but other parts of the codebase (e.g., Name.jsx and About.jsx) use "name" as the key, which may indicate an inconsistency.
Apply this diff:
function Welcome(){
- console.log(localStorage.getItem("profile"))
return(📝 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.
| console.log(localStorage.getItem("profile")) |
🤖 Prompt for AI Agents
In src/pages/WelcomePage.jsx around line 5, remove the debug
console.log(localStorage.getItem("profile")) statement; replace it with either
no logging or, if you need to read the stored value here, use the consistent key
used elsewhere (localStorage.getItem("name")) and assign it to a variable for
use in the component instead of logging, ensuring key names match across
Name.jsx and About.jsx.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/App.jsx (1)
24-39: Broken keyboard handler with React Router migration.The
currentPagestate is never updated by the router, so it always equals"home". The Escape key handler will always hide the window, never navigate. With React Router, you needuseLocationanduseNavigateto track and control navigation.Replace the keyboard handler with router-aware logic:
+ const location = useLocation(); + const navigate = useNavigate(); + useEffect(() => { function handleKeyDown(e) { if (e.key === "Escape") { - if (currentPage === "home") { + if (location.pathname === "/" || location.pathname === "/home") { getCurrentWindow().hide(); } else { - setCurrentPage("home"); - setQuery(""); - inputRef.current?.focus(); + navigate("/home"); } } } - + window.addEventListener("keydown", handleKeyDown); return () => window.removeEventListener("keydown", handleKeyDown); - }, [currentPage]); + }, [location.pathname, navigate]);Also add the import at the top:
-import { BrowserRouter, Routes, Route } from "react-router-dom"; +import { BrowserRouter, Routes, Route, useLocation, useNavigate } from "react-router-dom";Note: You'll need to move this
useEffectinto a child component ofBrowserRouter, since router hooks can't be called at theApplevel. Consider creating a<AppRoutes />component inside<BrowserRouter>.
♻️ Duplicate comments (9)
src/pages/WelcomePage.jsx (1)
13-13: Fix typo:felxshould beflex.The className contains a typo that will prevent the flex layout from applying.
Apply this diff:
- <div className="felx flex-row justify-center text-center text-3xl">A tool to make your life easy</div> + <div className="flex flex-row justify-center text-center text-3xl">A tool to make your life easy</div>src/pages/OpenFileGuide.jsx (3)
9-9: Replace invalid Tailwind classjustify-leftwithjustify-start.
justify-leftis not a valid Tailwind CSS utility. Usejustify-startfor left alignment in flexbox.Apply this diff:
- <div className="flex flex-row justify-left text-left text-4xl ">Open File:</div> + <div className="flex flex-row justify-start text-left text-4xl">Open File:</div>
13-13: Improve image accessibility with descriptive alt text.The alt text
'snapshot1'is not descriptive. Provide meaningful alternative text that describes what the image shows for screen reader users.Apply this diff:
- <img src={snapshort4} alt='snapshot1' className="rounded-t-xl "/> + <img src={snapshort4} alt="Screenshot showing the Open File feature with search results and file paths" className="rounded-t-xl"/>
15-15: Replace invalid Tailwind classleft-170with proper positioning.
left-170is not a valid Tailwind CSS utility. In Tailwind v4, use arbitrary values with parentheses syntax likeleft-(170px)or switch to standard spacing utilities.Apply this diff to use arbitrary value syntax:
- <Link to='/GuideEnd'><button type='button' className="relative bottom-3 left-170 flex flex-row justify-center py-1 px-3 bg-black rounded-md text-center text-white ">next</button></Link> + <Link to='/GuideEnd'><button type='button' className="relative bottom-3 left-(170px) flex flex-row justify-center py-1 px-3 bg-black rounded-md text-center text-white">next</button></Link>Alternatively, consider using flex layout utilities like
self-endorml-autofor more maintainable positioning.src/pages/ClipboardGuide.jsx (4)
8-8: Usemin-h-screenfor reliable full-page height.The container uses
h-full, which requires a parent with explicit height. Usemin-h-screento ensure the page reliably fills the viewport.- <div className="flex flex-col justify-center gap-5 p-5 h-full bg-[#3D3C3C] font-sans text-white"> + <div className="flex flex-col justify-center gap-5 p-5 min-h-screen bg-[#3D3C3C] font-sans text-white">
9-9: Replace invalid Tailwind classjustify-left.Tailwind v4 doesn't have a
justify-leftutility. Usejustify-startor remove it (sincetext-leftalready aligns text).- <div className="flex flex-row justify-left text-left text-4xl ">Clipboard:</div> + <div className="flex flex-row justify-start text-left text-4xl">Clipboard:</div>
14-15: Improve image alt text for accessibility.The alt attributes
'snapshot1'and'snapshot2'are not descriptive. Provide meaningful descriptions of what each screenshot shows.- <img src={snapshort1} alt='snapshot1' + <img src={snapshort1} alt="Clipboard manager showing saved entries with timestamps and usage counts" className="rounded-t-xl "/> @@ - <img src={snapshot2} alt='snapshot2' + <img src={snapshot2} alt="Clipboard search filtering entries by keyword in real-time" className="rounded-t-xl "/>Also applies to: 21-22
25-25: Add explicit button type and fix invalid Tailwind class.The button lacks
type="button"(a11y issue) and uses the invalid classleft-170. Use proper Tailwind utilities for positioning.- <Link to='/OnlineSearchGuide'><button className="relative bottom-3 left-170 flex flex-row justify-center py-1 px-3 bg-black rounded-md text-center text-white ">next</button></Link> + <Link to="/OnlineSearchGuide"> + <button type="button" className="self-end py-1 px-3 bg-black rounded-md text-white"> + Next + </button> + </Link>This uses
self-endto align the button to the right within the flex container (cleaner than relative positioning).src/App.jsx (1)
16-18: Guard the initial null state to prevent flash of wrong page.The commented console.log addresses part of the previous feedback, but the route at line 49 still evaluates
isFirstLaunchwhen it's initiallynull. Sincenullis falsy, the conditionalisFirstLaunch ? <Welcome/> : <Home/>will flash<Home/>before the hook resolves.Apply this diff to add a loading guard:
const isFirstLaunch = useFirstLaunch(); - // console.log("isFirstLaunch:", isFirstLaunch); - + + if (isFirstLaunch === null) { + return null; // or <div>Loading...</div> + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsonsrc-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
package.json(1 hunks)src-tauri/Cargo.toml(0 hunks)src-tauri/src/lib.rs(1 hunks)src/App.jsx(2 hunks)src/hooks/useFirstLaunch.jsx(1 hunks)src/pages/ClipboardGuide.jsx(1 hunks)src/pages/GuideEnd.jsx(1 hunks)src/pages/OnlineSearchGuide.jsx(1 hunks)src/pages/OpenFileGuide.jsx(1 hunks)src/pages/WelcomePage.jsx(1 hunks)vite.config.js(1 hunks)
💤 Files with no reviewable changes (1)
- src-tauri/Cargo.toml
🧰 Additional context used
🧬 Code graph analysis (2)
src/App.jsx (2)
src/hooks/useFirstLaunch.jsx (2)
isFirstLaunch(4-4)useFirstLaunch(3-18)src/pages/HomePage.jsx (3)
query(11-11)inputRef(12-12)currentPage(13-13)
src/hooks/useFirstLaunch.jsx (1)
src/App.jsx (1)
isFirstLaunch(16-16)
🪛 Biome (2.1.2)
src/pages/ClipboardGuide.jsx
[error] 25-25: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (4)
vite.config.js (1)
3-3: LGTM! Tailwind CSS v4 integration is correct.The Vite plugin integration follows the documented pattern for Tailwind CSS v4.
Also applies to: 9-9
package.json (1)
16-25: LGTM! Dependencies support the onboarding flow.The added packages (Tailwind CSS, React Router, react-hook-form) align with the PR objectives for implementing first-launch intro and navigation.
src-tauri/src/lib.rs (1)
240-240: No issues found — frontend does not invoke removed commands.Verification confirms the removed commands (
search_files,get_applications,get_recent_files,open_file,refresh_file_index) are not referenced anywhere in the frontend code. The frontend has been properly updated to use local data (dummy files in OpenFilePage, local OPTIONS array in HomeOptions) instead of backend command invocations. All current frontendinvoke()calls target existing clipboard-related commands that remain in the backend.src/App.jsx (1)
47-59: Duplicate route issue resolved.The duplicate
"/"route has been removed. The routing structure now correctly implements the first-launch conditional logic with a single root route.Optional: Consider using lowercase paths consistently (e.g.,
/aboutinstead of/About) to follow common URL conventions:- <Route path="/About" element={<About/>} /> - <Route path="/ClipboardGuide" element={<ClipboardGuide/>} /> - <Route path="/OnlineSearchGuide" element={<OnlineSearchGuide/>} /> - <Route path="/OpenFileGuide" element={<OpenFileGuide/>} /> - <Route path="/GuideEnd" element={<GuideEnd/>} /> + <Route path="/about" element={<About/>} /> + <Route path="/clipboard-guide" element={<ClipboardGuide/>} /> + <Route path="/online-search-guide" element={<OnlineSearchGuide/>} /> + <Route path="/open-file-guide" element={<OpenFileGuide/>} /> + <Route path="/guide-end" element={<GuideEnd/>} />Update the corresponding
<Link to="...">paths in other components if you make this change.
| const [query, setQuery] = useState(""); | ||
| const inputRef = useRef(null); | ||
| const [currentPage, setCurrentPage] = useState("home"); | ||
|
|
||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused state declarations.
The query, inputRef, and currentPage state are no longer used in the returned JSX after migrating to React Router. These should either be removed or moved to the relevant page components (e.g., HomePage) where they're actually needed.
Apply this diff to remove the unused state:
- const [query, setQuery] = useState("");
- const inputRef = useRef(null);
- const [currentPage, setCurrentPage] = useState("home");
- Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/App.jsx around lines 20–23 remove the now-unused declarations "const
[query, setQuery] = useState(\"\"); const inputRef = useRef(null); const
[currentPage, setCurrentPage] = useState(\"home\");" and any references to them;
if the functionality is required, move the state and ref into the specific page
component (e.g., HomePage) and update that component to manage
query/inputRef/currentPage instead; also remove unused imports (useState,
useRef) from the top of App.jsx to silence lint errors and run the app to
confirm no remaining references.
| useEffect(() => { | ||
| inputRef.current?.focus(); | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove focus effect for non-existent ref.
The inputRef is not rendered in the App component's JSX, so this useEffect does nothing. Move this focus logic to the component that actually renders the input (likely HomePage).
- useEffect(() => {
- inputRef.current?.focus();
- }, []);
-📝 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.
| useEffect(() => { | |
| inputRef.current?.focus(); | |
| }, []); |
🤖 Prompt for AI Agents
In src/App.jsx around lines 41 to 43, the useEffect that calls
inputRef.current?.focus() runs against a ref that is not rendered in this
component; move the focus logic to the component that actually mounts the input
(e.g., HomePage). Remove the useEffect from App.jsx, and add an equivalent
useEffect inside the input-owning component that references its local ref
(create ref there if needed) so focus is applied when that component mounts.
| useEffect(() => { | ||
| const flag = localStorage.getItem("firstLaunch"); | ||
|
|
||
| if (flag === null) { | ||
| localStorage.setItem("firstLaunch", "true"); | ||
| setIsFirstLaunch(true); | ||
| } else { | ||
| setIsFirstLaunch(false); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider clearer flag semantics for the first-launch state.
The current implementation sets localStorage.setItem("firstLaunch", "true") on first launch, but then returns false when this flag exists on subsequent launches. This is functionally correct but semantically confusing—the stored value "true" doesn't align with the returned false.
For better clarity, consider storing "completed" or "false" to indicate the first launch has already occurred:
if (flag === null) {
- localStorage.setItem("firstLaunch", "true");
+ localStorage.setItem("firstLaunch", "completed");
setIsFirstLaunch(true);
} else {
setIsFirstLaunch(false);
}Or use a more descriptive key name like "hasSeenWelcome" to better express intent.
📝 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.
| useEffect(() => { | |
| const flag = localStorage.getItem("firstLaunch"); | |
| if (flag === null) { | |
| localStorage.setItem("firstLaunch", "true"); | |
| setIsFirstLaunch(true); | |
| } else { | |
| setIsFirstLaunch(false); | |
| } | |
| useEffect(() => { | |
| const flag = localStorage.getItem("firstLaunch"); | |
| if (flag === null) { | |
| localStorage.setItem("firstLaunch", "completed"); | |
| setIsFirstLaunch(true); | |
| } else { | |
| setIsFirstLaunch(false); | |
| } |
🤖 Prompt for AI Agents
In src/hooks/useFirstLaunch.jsx around lines 6 to 14, the hook writes
localStorage.setItem("firstLaunch", "true") on first run but then treats the
mere existence of the key as "not first launch", which is semantically
confusing; change the stored value to reflect that the onboarding was completed
(e.g., set "firstLaunch" to "completed" or "false") or rename the key to a
clearer name like "hasSeenWelcome", and update the getItem/ setItem checks so
the stored value meaning matches the boolean returned by setIsFirstLaunch (e.g.,
treat missing key as first launch, on first run set key to "completed" or
"trueSeen", and on subsequent loads read that value to setIsFirstLaunch(false)).
| import snapshort1 from '../assets/snapshot1.png' | ||
| import snapshot2 from '../assets/snapshort2.png' |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix inconsistent image import naming.
Line 2 imports snapshot1.png as snapshort1 (typo in the variable), while line 3 imports snapshort2.png (typo in filename) as snapshot2. This inconsistency is confusing.
Standardize the naming:
-import snapshort1 from '../assets/snapshot1.png'
-import snapshot2 from '../assets/snapshort2.png'
+import snapshot1 from '../assets/snapshot1.png'
+import snapshot2 from '../assets/snapshot2.png'Then update line 14 to use snapshot1:
- <img src={snapshort1} alt='snapshot1'
+ <img src={snapshot1} alt='snapshot1'(Also rename the actual file snapshort2.png to snapshot2.png if needed.)
📝 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.
| import snapshort1 from '../assets/snapshot1.png' | |
| import snapshot2 from '../assets/snapshort2.png' | |
| import snapshot1 from '../assets/snapshot1.png' | |
| import snapshot2 from '../assets/snapshot2.png' |
🤖 Prompt for AI Agents
In src/pages/ClipboardGuide.jsx around lines 2–3, the image imports have
inconsistent/typo'd names: line 2 imports snapshot1.png as "snapshort1" and line
3 imports "snapshort2.png" as snapshot2; rename the import variables to be
consistent (import snapshot1 from '../assets/snapshot1.png' and import snapshot2
from '../assets/snapshot2.png') and update line 14 to use snapshot1 instead of
snapshort1; also ensure the asset filename is corrected on disk (rename
snapshort2.png → snapshot2.png) so the import paths match.
| <div className="text-center text-5xl"> | ||
| All done, continue to | ||
| </div> |
There was a problem hiding this comment.
Add text color for visibility on dark background.
The text "All done, continue to" lacks a color specification and will render in default black on the dark gray background (bg-[#3D3C3C]), making it invisible or barely visible.
Apply this diff to add text color:
- <div className="text-center text-5xl">
+ <div className="text-center text-5xl text-white">
All done, continue to
</div>📝 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.
| <div className="text-center text-5xl"> | |
| All done, continue to | |
| </div> | |
| <div className="text-center text-5xl text-white"> | |
| All done, continue to | |
| </div> |
🤖 Prompt for AI Agents
In src/pages/GuideEnd.jsx around lines 8 to 10 the heading div lacks a text
color and will render as black on the dark gray background; add an explicit text
color class (e.g., text-white or text-neutral-100) to the div (or its parent) so
the text is visible on bg-[#3D3C3C], keeping existing typography classes.
| function OnlineSearchGuide(){ | ||
| return( | ||
| <div className="flex flex-col justify-center gap-5 p-5 h-full bg-[#3D3C3C] font-sans text-white"> | ||
| <div className="flex flex-row justify-left text-left text-4xl ">Online Search:</div> |
There was a problem hiding this comment.
Replace invalid Tailwind class justify-left with justify-start.
justify-left is not a valid Tailwind CSS utility. Use justify-start for left alignment in flexbox.
Apply this diff:
- <div className="flex flex-row justify-left text-left text-4xl ">Online Search:</div>
+ <div className="flex flex-row justify-start text-left text-4xl">Online Search:</div>📝 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.
| <div className="flex flex-row justify-left text-left text-4xl ">Online Search:</div> | |
| <div className="flex flex-row justify-start text-left text-4xl">Online Search:</div> |
🤖 Prompt for AI Agents
In src/pages/OnlineSearchGuide.jsx around line 8, the Tailwind class
"justify-left" is invalid; replace it with "justify-start" so the div uses the
correct flexbox left alignment utility. Update the className string accordingly
to remove "justify-left" and add "justify-start" (keeping the other classes
unchanged).
| <img src={snapshot3} alt='snapshot1' | ||
| className="flex justify-center items-center rounded-t-xl "/> | ||
| </div> | ||
| <Link to='/OpenFileGuide'><button type="button" className="relative bottom-3 left-170 flex flex-row justify-center py-1 px-3 bg-black rounded-md text-center text-white ">next</button></Link> |
There was a problem hiding this comment.
Replace invalid Tailwind class left-170 with proper positioning.
left-170 is not a valid Tailwind CSS utility. In Tailwind v4, use arbitrary values with parentheses syntax like left-(170px) or switch to margin-based layout utilities.
Apply this diff to use arbitrary value syntax:
- <Link to='/OpenFileGuide'><button type="button" className="relative bottom-3 left-170 flex flex-row justify-center py-1 px-3 bg-black rounded-md text-center text-white ">next</button></Link>
+ <Link to='/OpenFileGuide'><button type="button" className="relative bottom-3 left-(170px) flex flex-row justify-center py-1 px-3 bg-black rounded-md text-center text-white">next</button></Link>Alternatively, use standard spacing utilities or consider using ml-auto if positioning the button to the right within a flex container.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/OnlineSearchGuide.jsx around line 15, the Tailwind class "left-170"
is invalid; replace it with a valid positioning utility such as using Tailwind's
arbitrary value syntax (e.g., left-(170px)) or remove absolute left positioning
and use layout utilities like ml-auto or justify-end on the flex container to
push the button to the right; update the className accordingly and ensure the
element has a positioning context (e.g., relative/absolute) if you keep
left-(170px).
|
Hey @AlokPy1484 |
Larry8668
left a comment
Larry8668
left a comment
There was a problem hiding this comment.
Take a pull from main and rebase
you can download the latest release and check how the search for apps works
all the code with regards to that has been removed in this PR
@AlokPy1484
There was a problem hiding this comment.
the code for accessing apps on system has been removed
There was a problem hiding this comment.
the code for accessing apps on system has been removed
There was a problem hiding this comment.
the code for accessing apps on system has been removed
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/pages/Name.jsx (1)
26-32: Form submission is bypassed by the external navigation button.The form has an
onSubmithandler that saves the name to localStorage (lines 16-20), but the "next" button (line 32) is wrapped in a Link that navigates directly to/About, bypassing form submission entirely. This means:
- Clicking "next" navigates without saving the name
- Users proceed to the About page with no name stored in localStorage
- The form's validation and submit logic never executes
Apply this diff to move the button inside the form:
<div> <form onSubmit={handleSubmit(onSubmit)}> <input className="bg-white rounded-md text-2xl text-center" defaultValue="" {...register("name")} type='text' placeholder="Type"/> + <button type="submit" className="absolute bottom-3 right-5 flex flex-row justify-center py-1 px-3 bg-black rounded-md text-center text-white ">next</button> </form> </div> - <Link to='/About'><button type='submit' className="absolute bottom-3 right-5 flex flex-row justify-center py-1 px-3 bg-black rounded-md text-center text-white ">next</button></Link>Note: The
navigate("/About")inonSubmitalready handles navigation, so the Link is unnecessary.
| const { | ||
| register, | ||
| handleSubmit, | ||
| formState: { errors }, | ||
| } = useForm() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unused errors destructuring.
The errors object from formState is destructured but never used in the component.
Apply this diff:
const {
register,
handleSubmit,
- formState: { errors },
} = useForm()📝 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 { | |
| register, | |
| handleSubmit, | |
| formState: { errors }, | |
| } = useForm() | |
| const { | |
| register, | |
| handleSubmit, | |
| } = useForm() |
🤖 Prompt for AI Agents
In src/pages/Name.jsx around lines 10 to 14, the formState.errors value is
destructured but never used; remove the unused `errors` from the destructuring
to clean up the code (i.e., change the useForm destructure to only pull register
and handleSubmit) so the component no longer declares an unused variable.
| const onSubmit = (data) => { | ||
| localStorage.setItem("name", JSON.stringify(data)) | ||
| // console.log(localStorage.getItem("name")) | ||
| navigate("/About"); | ||
| } |
There was a problem hiding this comment.
Add input validation and error handling.
The function saves data to localStorage without validation or error handling, which can lead to issues:
- Users can submit empty or whitespace-only names
- localStorage operations can throw exceptions (e.g., in private browsing mode or when quota is exceeded)
Apply this diff to add validation and error handling:
const onSubmit = (data) => {
+ if (!data.name || !data.name.trim()) {
+ // Handle empty name - could show error message
+ return;
+ }
+ try {
localStorage.setItem("name", JSON.stringify(data))
- // console.log(localStorage.getItem("name"))
navigate("/About");
+ } catch (error) {
+ console.error("Failed to save name to localStorage:", error);
+ // Handle error - could show error message to user
+ }
}Also consider configuring react-hook-form validation:
<input
className="bg-white rounded-md text-2xl text-center"
- defaultValue="" {...register("name")} type='text' placeholder="Type"/>
+ defaultValue="" {...register("name", { required: true, validate: value => value.trim().length > 0 })} type='text' placeholder="Type"/>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/Name.jsx around lines 16 to 20, the onSubmit handler writes to
localStorage without validating the name or handling storage errors; update it
to trim the input and reject empty or whitespace-only names (set a form error or
display a message), add a try/catch around localStorage.setItem to
handle/storage quota or private-mode exceptions, only call navigate('/About')
when storage succeeds, and surface any error to the user; additionally,
configure react-hook-form validation rules for the name field (required,
minLength, pattern to disallow only whitespace) so invalid submissions are
blocked at the form level.
| <input | ||
| className="bg-white rounded-md text-2xl text-center" | ||
| defaultValue="" {...register("name")} type='text' placeholder="Type"/> |
There was a problem hiding this comment.
Add a label for accessibility.
The input field lacks an associated label, making it inaccessible to screen reader users and reducing usability for all users.
Apply this diff to add a proper label:
- <div className="text-4xl">Enter Your Name</div>
<div>
<form onSubmit={handleSubmit(onSubmit)}>
+ <label htmlFor="name-input" className="text-4xl block mb-5">Enter Your Name</label>
<input
+ id="name-input"
className="bg-white rounded-md text-2xl text-center"
defaultValue="" {...register("name")} type='text' placeholder="Type"/>
</form>
</div>Alternatively, you can use aria-label:
<input
+ aria-label="Enter Your Name"
className="bg-white rounded-md text-2xl text-center"
defaultValue="" {...register("name")} type='text' placeholder="Type"/>📝 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.
| <input | |
| className="bg-white rounded-md text-2xl text-center" | |
| defaultValue="" {...register("name")} type='text' placeholder="Type"/> | |
| <input | |
| aria-label="Enter Your Name" | |
| className="bg-white rounded-md text-2xl text-center" | |
| defaultValue="" {...register("name")} type='text' placeholder="Type"/> |
🤖 Prompt for AI Agents
In src/pages/Name.jsx around lines 27-29, the text input has no accessible
label; add one by giving the input an id (e.g., id="name") and inserting a
corresponding <label htmlFor="name">Name</label> immediately before the input
(or, if you prefer not to render a visible label, add an aria-label="Name"
attribute to the input instead); ensure the label text is descriptive and that
the id used by htmlFor matches the input id.
There was a problem hiding this comment.
@Larry8668 I have tried to restore fuzzy search code, see if this commit is ok.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
frontend seems to be back, backend is still missing |
I realise that that these issues are arising cause I wrote the code of my new feature in base file of a prev version of this projects due to which the fuzzy search feature was not present in the new PR. I have gone through each file carefully and compared any change that might remove existing feature and tried to eliminate it in my PR with a new branch to keep the Pull Request clean. Hence, I am closing this PR |
2 participants
**This PR aims to Closes #10 Add an intro to the app on first time launch **
Objectives-
My Approach-
I was able to achieve all the objective with the approach we agreed on.
New library used-
This PR aims to close #6 Add an intro to the app on first time launch #10 only, will add the setting feature in next PR.
Summary by CodeRabbit
New Features
Changes