- 
                Notifications
    
You must be signed in to change notification settings  - Fork 145
 
Remove career page and references of it from the app #2686
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: master
Are you sure you want to change the base?
Conversation
| 
           The latest updates on your projects. Learn more about Vercel for Git ↗︎ 
  | 
    
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 removes the careers/jobs functionality from the PolicyEngine application by deleting the Jobs page component and all references to it throughout the codebase. This addresses issue #2677 and simplifies the application by removing job-related navigation and content.
- Completely removes the Jobs page component and its associated job data
 - Updates navigation by removing careers link from the header dropdown menu
 - Cleans up references to the jobs page in routing and the About page
 
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description | 
|---|---|
| src/pages/Jobs.jsx | Complete removal of the Jobs page component | 
| src/pages/About.jsx | Removes link to jobs page from the team description | 
| src/layout/Header.jsx | Removes "Careers" entry from the about dropdown navigation | 
| src/tests/pages/About.test.js | Removes test assertions for the jobs page link | 
| src/PolicyEngine.jsx | Removes Jobs import and route configuration | 
| public/data/job-openings.json | Removes job openings data file | 
| 
           Let's keep the structure just clear out the contents of the job openings json file, and say "PolicyEngine does not currently have job openings. Please check back later." if it's empty  | 
    
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.
Thanks for this @SongmingFan123. There are a few minor removals, then this will be good to go.
| title: "Team", | ||
| link: "about", | ||
| icon: <TeamOutlined />, | ||
| }, | 
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.
issue, minor: Please remove the unused import on line 26 for UserAddOutlined
I would've added above, but GH doesn't allow it
        
          
                src/pages/About.jsx
              
                Outdated
          
        
      | > | ||
| Learn about opportunities to join us. | ||
| </Link> | ||
| contributors. | 
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.
issue, minor: Please remove the unused import of Link on line 8 and the code defining countryId on line 14.
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.
Updated, thanks
          
 Should I restore the Jobs page and keep the message when the job openings json file is empty?  | 
    
| 
           Sorry, I flipped through this too quickly and didn't read Max's comment. Please ignore all of my review.  | 
    
| 
           Please drop a screenshot  | 
    
| 
           Per the issue please remove internship applications too 
  | 
    
          
 Updated  | 
    
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.
@SongmingFan123 a few minor changes requested. Please also address the ESLint error.
| // If the path is /, redirect to /[countryId] | ||
| // If the path is /[countryId], render the homepage | ||
| // If the path is /[countryId]/about, render the about page | ||
| // If the path is /[countryId]/jobs, render the jobs page | 
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.
minor: This should be added back if the page is being rendered
| icon: <TeamOutlined />, | ||
| }, | ||
| { | ||
| title: "Careers", | 
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.
issue, blocking?: Please revert this to "Careers," not "Jobs", and change the icon back.
| const location = useLocation(); | ||
| const pathParts = location.pathname.split("/"); | ||
| const countryId = pathParts[1]; // Assumes the countryId is always the second segment in the path | ||
| const countryId = useCountryId(); | 
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.
accolade: Thanks for using this hook


Fixes #2677