-
Notifications
You must be signed in to change notification settings - Fork 355
Retrigger search projects in home page #1350
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Thank you for submitting your pull request! We'll review it as soon as possible. For further communication, join our discord server https://discord.gg/tSqtvHUJzE. |
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 search functionality and restructures the application by moving the app directory from modules to the src level. The search feature has been reimplemented to work as a query parameter-based module within the home page, replacing the previous route-based approach. The changes introduce a more integrated search experience with improved state management.
Changes:
- Relocated
appdirectory frommodules/apptosrc/appwith corresponding import path updates across 20+ files - Replaced the standalone
/search/:queryroute with a query parameter-based search (?searchTerm=...) integrated into the home page - Extracted home page content into a separate
HomeContentcomponent for better code organization - Enhanced Navbar component to support external search state management through props
Reviewed changes
Copilot reviewed 33 out of 36 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/app/routes/* | Newly moved app routing infrastructure with updated relative imports |
| client/src/app/components/index.tsx | Updated component lazy loading paths to reflect new module structure |
| client/src/modules/home/v1/index.tsx | Refactored to conditionally render search or home content based on query params |
| client/src/modules/home/v1/hooks/index.ts | Added searchTerm and activeModule state management using query parameters |
| client/src/modules/home/v1/components/index.tsx | New HomeContent component extracted from main home page |
| client/src/modules/home/routes/index.tsx | Renamed function to HOME_ROUTES and added HOME_QUERY_PARAMS enum |
| client/src/modules/home/modules/search/v1/* | New search module implementation with dedicated hooks and components |
| client/src/modules/search/* | Old search module removed completely |
| client/src/shared/components/organisms/navbar/* | Enhanced to support controlled search state from parent components |
| client/src/modules/settings/* | Updated imports to reference relocated app directory |
| client/src/modules/notification/* | Updated imports to reference relocated app directory |
| client/src/modules/manage-projects/* | Updated imports to reference relocated app directory |
| client/src/App.tsx | Updated import paths for relocated app routing modules |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@Avdhesh-Varshney I've opened a new pull request, #1351, to work on those changes. Once the pull request is ready, I'll request review from you. |
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
Copilot reviewed 41 out of 45 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
client/src/shared/components/organisms/header/hooks/index.ts:79
- The
useEffecthook is missingtriggerSearchByKeyboardin its dependency array. This function is defined inside the component and usesisDesktopandsearchInputRef, so it should either be added to the dependencies or wrapped in auseCallbackhook to ensure the effect has stable dependencies.
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Pull Requests Review Criteria
Caution
PRs that fail to meet these review standards will be automatically flagged and may be rejected by maintainers.
mainDescribe the add-ons or changes you've made 📃
appdirectory from modules to src folder