-
Notifications
You must be signed in to change notification settings - Fork 3
Saved opportunities integration, individual opportunity view, opportunity context rework #179
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
base: main
Are you sure you want to change the base?
Conversation
modified: src/individuals/pages/Saved.tsx modified: src/opportunities/components/OpportunitiesDetails.tsx modified: src/opportunities/pages/Opportunities.tsx modified: src/shared/components/Navigation/MainNavigation.tsx
modified: src/opportunities/components/Posts.tsx
modified: src/opportunities/components/FiltersField.tsx modified: src/opportunities/components/OpportunitiesDetails.tsx modified: src/opportunities/components/Posts.tsx deleted: src/opportunities/components/SearchBar.tsx
modified: src/opportunities/components/FiltersField.tsx renamed: src/opportunities/components/OpportunitiesDetails.tsx -> src/opportunities/components/OpportunitiesList.tsx modified: src/opportunities/components/PopUpMenu.tsx modified: src/opportunities/components/Posts.tsx modified: src/opportunities/pages/Opportunities.tsx modified: src/shared/components/Checkbox.tsx new file: src/types/filters.ts deleted: src/types/opportunities.ts modified: src/types/opportunity.ts new file: src/types/opportunityaction.ts
new file: src/context/useOpportunity.tsx modified: src/opportunities/components/FiltersField.tsx modified: src/opportunities/components/OpportunitiesList.tsx modified: src/opportunities/components/PopUpMenu.tsx modified: src/opportunities/components/Posts.tsx modified: tsconfig.json
new file: docs/posters/labonnect_S25_poster (1).pdf
deleted: docs/posters/labconnect_F25_poster.png
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 opportunities feature by centralizing state management using React Context, integrating saved opportunities into the main Opportunities page, and consolidating type definitions. The changes remove the standalone /saved route and instead provide a tabbed interface on the Opportunities page to switch between "Search" and "Saved" views.
Key Changes:
- Introduced
OpportunityContextwithuseReducerfor centralized opportunity state management (filters, query, opportunities list) - Integrated saved opportunities as a tab within the Opportunities page, removing the separate
/savedroute - Refactored type definitions by splitting
opportunities.tsinto separatefilters.tsand updatedopportunity.tsfiles
Reviewed changes
Copilot reviewed 16 out of 19 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
tsconfig.json |
Changed moduleResolution from "node" to "bundler" for improved module resolution |
src/types/opportunityaction.ts |
New file defining action types for the opportunity reducer |
src/types/opportunity.ts |
Updated type definition with consistent property types and added saved field |
src/types/opportunities.ts |
Removed file, split into separate type files |
src/types/filters.ts |
New file containing the Filters type extracted from opportunities.ts |
src/shared/components/Navigation/MainNavigation.tsx |
Removed "Saved" navigation link |
src/shared/components/Checkbox.tsx |
Updated import path and removed "semesters" filter check |
src/opportunities/pages/Opportunities.tsx |
Wrapped with OpportunityProvider and integrated SavedPage as a tab |
src/opportunities/components/SearchBar.tsx |
Removed standalone SearchBar component |
src/opportunities/components/Posts.tsx |
Refactored to use OpportunityContext instead of local reducer |
src/opportunities/components/PopUpMenu.tsx |
Updated to consume context instead of receiving props |
src/opportunities/components/OpportunitiesList.tsx |
Added save/unsave functionality with API integration |
src/opportunities/components/FiltersField.tsx |
Integrated inline search form and moved to context-based state |
src/individuals/pages/Saved.tsx |
Updated styling to match OpportunitiesList table design |
src/context/useOpportunity.tsx |
New custom hook for accessing OpportunityContext |
src/context/OpportunityContext.tsx |
New context provider managing opportunity state with useReducer |
src/App.tsx |
Removed /saved route |
Comments suppressed due to low confidence (6)
src/opportunities/components/OpportunitiesList.tsx:4
- Unused import
OpportunityAction. This should be removed if it's not needed.
src/opportunities/components/OpportunitiesList.tsx:1 - Unused imports
useStateanduseEffect. These should be removed if they're not needed.
src/opportunities/components/OpportunitiesList.tsx:64 - The optimistic update in the
finallyblock is incorrect. It should happen before the API call (optimistic), not after. Currently, if the API call fails, the UI still updates. The update should either happen before the fetch (true optimistic) with a rollback on error, or after a successful response (pessimistic). The current implementation updates the UI regardless of success or failure.
src/opportunities/components/OpportunitiesList.tsx:24 - Multiple
console.logstatements should be removed from production code. Found at lines 14, 20, 24, 27, 41, 54, and 63.
src/opportunities/components/OpportunitiesList.tsx:1 - Unused imports useEffect, useState.
src/opportunities/components/OpportunitiesList.tsx:4 - Unused import OpportunityAction.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| id={item} | ||
| className={type === "radio" ? "radio" : "checkbox"} | ||
| defaultChecked={(name === "semesters" && filters?.semesters?.includes(item)) || (name === "years" && filters?.years?.includes(Number(item))) || (name === "credits" && filters?.credits?.includes(item) ? true : false)} | ||
| defaultChecked={(name === "years" && filters?.years?.includes(Number(item))) || (name === "credits" && filters?.credits?.includes(item) ? true : false)} |
Copilot
AI
Dec 10, 2025
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.
The ternary operator logic is incorrect. The expression (name === "years" && filters?.years?.includes(Number(item))) || (name === "credits" && filters?.credits?.includes(item) ? true : false) has incorrect operator precedence. The ternary operator binds to only the second condition, not the entire OR expression. This should be: ((name === "years" && filters?.years?.includes(Number(item))) || (name === "credits" && filters?.credits?.includes(item))) ? true : false or simply (name === "years" && filters?.years?.includes(Number(item))) || (name === "credits" && filters?.credits?.includes(item)).
| defaultChecked={(name === "years" && filters?.years?.includes(Number(item))) || (name === "credits" && filters?.credits?.includes(item) ? true : false)} | |
| defaultChecked={(name === "years" && filters?.years?.includes(Number(item))) || (name === "credits" && filters?.credits?.includes(item))} |
| } catch { | ||
| console.log("Error fetching saved"); | ||
| } | ||
| console.log(saved) |
Copilot
AI
Dec 10, 2025
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.
Debug console.log statements should be removed from production code (lines 32 and 36).
| activeFilters: ["2025"], | ||
| filterMap: { years: [2025], credits: [], hourlyPay: 0, majors: [] }, |
Copilot
AI
Dec 10, 2025
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.
The initial state hardcodes "2025" and [2025] instead of using currYr (defined on line 7), which means the initial filters won't update when the year changes. Replace "2025" with currYr.toString() and [2025] with [currYr] for consistency with the RESET_FILTERS action.
| activeFilters: ["2025"], | |
| filterMap: { years: [2025], credits: [], hourlyPay: 0, majors: [] }, | |
| activeFilters: [currYr.toString()], | |
| filterMap: { years: [currYr], credits: [], hourlyPay: 0, majors: [] }, |
| useEffect(() => { | ||
| fetchSaved(); | ||
| }, []); |
Copilot
AI
Dec 10, 2025
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.
The useEffect has an empty dependency array but should include fetchSaved in its dependencies, or better yet, define fetchSaved inside the useEffect to avoid the stale closure issue. Additionally, the fetchSaved function should be wrapped with useCallback if it remains outside.
| activeFilters: string[]; | ||
| filterMap: Filters; | ||
| }; | ||
| query: string; |
Copilot
AI
Dec 10, 2025
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.
The query field in state is set but never exposed through the context provider. It should be added to the OpportunityContextType interface and the context provider value to be usable by consuming components.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Saved opportunities integrated into Opportunities page. Also integrated save and unsave buttons. Individual opportunity view tied to profile view and opportunity view buttons with backend route integration. OpportunityContext reowrk for all of the oppoortunity components.
Type of change
How Has This Been Tested?
Rigorously on local machine
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
Checklist: