-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add roadmap feature to landing page + small fixes #574
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
Introduces a new Roadmap component and related operations to the landing page, adds a getGithubRoadmap query, and updates navigation constants. Also updates several dependencies and lockfile entries, including TailwindCSS and React-related packages.
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 introduces a dynamic Roadmap feature to the OpenSaaS landing page that fetches and displays GitHub project data, along with several UI improvements including fixed navbar scrolling and hero section overflow behavior.
Key Changes
- Added new
getGithubRoadmapquery that fetches epic data from GitHub's GraphQL API with client-side caching - Created a responsive Roadmap component displaying epics in a kanban-style grid organized by status (Ideas, Planned, In Progress, Done)
- Fixed navbar scrolling behavior to account for fixed header offset and smooth scroll to top when navigating to home
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| opensaas-sh/app_diff/src/landing-page/operations.ts.diff | New server-side operation for fetching GitHub roadmap data with Zod validation and in-memory caching |
| opensaas-sh/app_diff/src/landing-page/components/Roadmap.tsx.diff | New Roadmap component with kanban-style UI displaying epics with progress bars |
| opensaas-sh/app_diff/src/client/App.tsx.diff | Updated scroll behavior with offset for fixed navbar and smooth scrolling |
| opensaas-sh/app_diff/src/client/components/NavBar/constants.ts.diff | Updated navigation items to link to auth feature and roadmap sections |
| opensaas-sh/app_diff/src/landing-page/LandingPage.tsx.diff | Integrated Roadmap component into landing page layout |
| opensaas-sh/app_diff/src/landing-page/components/Hero/Hero.tsx.diff | Fixed overflow-x behavior for hero section orbit animation |
| opensaas-sh/app_diff/src/landing-page/components/HighlightedFeature.tsx.diff | Added id prop support for anchor linking |
| opensaas-sh/app_diff/src/landing-page/components/Examples/Auth.tsx.diff | Added id to auth feature for navigation |
| opensaas-sh/app_diff/main.wasp.diff | Registered new getGithubRoadmap query |
| opensaas-sh/app_diff/package-lock.json.diff | Updated dependencies including tailwindcss and added dev dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| + <div className={`flex items-center justify-center p-3 rounded-lg border ${borderColor} ${statusColor.split(' ')[0]} bg-opacity-20`}> | ||
| + <h3 className={`font-bold ${statusColor.split(' ')[1]}`}> |
Copilot
AI
Nov 20, 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.
Using .split(' ') to extract parts of className strings is fragile and error-prone. This approach breaks if the order of classes changes or if additional classes are added. Consider extracting the specific color classes into separate helper functions or using a more robust method to compose classes.
| + href={epic.url} | ||
| + target="_blank" | ||
| + rel="noreferrer" | ||
| + className={`group flex flex-col gap-3 rounded-xl border p-5 transition-all hover:shadow-md border-gray-200 dark:border-gray-700 bg-white dark:bg-gray-900/50 ${hoverBorderColor} block`} |
Copilot
AI
Nov 20, 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.
[nitpick] The className has redundant block at the end since <a> is already an inline element and flex class is already applied. The block class is unnecessary and could be removed.
| + className={`group flex flex-col gap-3 rounded-xl border p-5 transition-all hover:shadow-md border-gray-200 dark:border-gray-700 bg-white dark:bg-gray-900/50 ${hoverBorderColor} block`} | |
| + className={`group flex flex-col gap-3 rounded-xl border p-5 transition-all hover:shadow-md border-gray-200 dark:border-gray-700 bg-white dark:bg-gray-900/50 ${hoverBorderColor}`} |
| + variables: { org: ORG_NAME, number: PROJECT_NUMBER }, | ||
| + }), | ||
| + }) | ||
| + |
Copilot
AI
Nov 20, 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 fetch response status is not checked before parsing JSON. If the GitHub API returns a non-200 status (e.g., 401, 403, 500), response.json() will still attempt to parse the response body, which may not be valid JSON or may contain error information. Consider checking response.ok or response.status before parsing the JSON.
| + | |
| + | |
| + if (!response.ok) { | |
| + // Try to parse error message if available | |
| + let errorMessage = `GitHub API request failed with status ${response.status}: ${response.statusText}` | |
| + try { | |
| + const errorJson = await response.json() | |
| + if (errorJson && errorJson.message) { | |
| + errorMessage += ` - ${errorJson.message}` | |
| + } | |
| + } catch (e) { | |
| + // Ignore JSON parse errors, use default error message | |
| + } | |
| + console.error(errorMessage) | |
| + return cachedData || [] | |
| + } | |
| + |
| + } else if (location.pathname === "/") { | ||
| + // scroll to top of page | ||
| + if (window.scrollY > 0) { | ||
| + console.log("scrolling to top"); |
Copilot
AI
Nov 20, 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.
Remove debug console.log statement. This should not be left in production code.
| + console.log("scrolling to top"); |
FranjoMindek
left a comment
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.
I really like this Roadmap feature.
Very nice addition.
There is no real logical errors here, just code placement / naming suggestions.
| +import type { GetGithubRoadmap } from 'wasp/server/operations' | ||
| +import * as z from 'zod' | ||
| + | ||
| +const ORG_NAME = "wasp-lang" |
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.
GITHUB_ORG_NAME
| +import * as z from 'zod' | ||
| + | ||
| +const ORG_NAME = "wasp-lang" | ||
| +const PROJECT_NUMBER = 6 |
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.
OPEN_SAAS_GITHUB_PROJECT_NUMBER
| + errors: z.array(z.any()).nullish() | ||
| +}) | ||
| + | ||
| +export enum EpicStatus { |
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.
GithubEpicStatus
| + url: string | ||
| +} | ||
| + | ||
| +type RoadmapData = EpicStats[] |
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.
I don't think we really need this.
Inline is fine.
e.g. export const getGithubRoadmap: GetGithubRoadmap<void, GithubEpicStats[]> = async () => {
| + const borderColor = getBorderColor(status); | ||
| + const statusColor = getStatusColor(status); | ||
| + | ||
| + return ( |
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.
Ideally we would extra these two into separate components.
e.g. RoadmapStatusColumn and RoadmapEpic
| + | ||
| + return ( | ||
| + <div key={status} className="flex flex-col gap-4"> | ||
| + <div className={`flex items-center justify-center p-3 rounded-lg border ${borderColor} ${statusColor.split(' ')[0]} bg-opacity-20`}> |
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.
Something here with statusColor is wrong.
We only ever take the first and second class out of the statusColor. Why do we define the rest of it then? Should it actually be getStatusBackground and getStatusTextColor?
Any way all of these issues would be a bit nicer to solve if you extracted the two subcomponents you're rendering into separate components.
Maybe the solution is to simplify the number of different color variants. e.g. only 2 blues/greens... used
| + rel="noreferrer" | ||
| + className={`group flex flex-col gap-3 rounded-xl border p-5 transition-all hover:shadow-md border-gray-200 dark:border-gray-700 bg-white dark:bg-gray-900/50 ${hoverBorderColor} block`} | ||
| + > | ||
| + <h4 className="font-semibold text-base text-gray-900 dark:text-white leading-tight group-hover:opacity-80 transition-opacity"> |
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.
Why opacity-80 on hover? Makes it seem diluted instead of "selected".
| + </div> | ||
| + <div className="h-1.5 w-full rounded-full bg-gray-100 dark:bg-gray-800 overflow-hidden"> | ||
| + <div | ||
| + className={`h-full rounded-full transition-all duration-500 ${progressColor}`} |
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.
I guess this transition will only trigger if we re-fetch the data and we solved some issue.
Seems very rare hahaha, not that I mind it.
| + } else if (location.pathname === "/") { | ||
| + // scroll to top of page | ||
| + if (window.scrollY > 0) { | ||
| + console.log("scrolling to top"); |
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.
Yep, we don't need this console.log
Added new roadmap-related images to the assets and banner directories. Updated the 2025-11-21 blog post to include these images, improve structure, and provide more details about the public roadmap, guiding themes, upcoming features, and ways to participate in the Open SaaS project.
Revised the 2025-11-21 roadmap blog post for clarity and improved messaging, added a tip section about subscribing to GitHub epics, and included a new 'subscribe-github.webp' image asset.
Reworked roadmap UI by splitting logic into RoadmapEpicCard and RoadmapStatusColumn components. Updated roadmap data model and operations to use GithubEpic and GithubEpicStatus, improving type safety and maintainability. Enhanced roadmap fetching and caching logic, and simplified App.tsx scroll behavior.
roadmap-lp.mp4
Description
Introduces a new Roadmap component and related operations to the landing page, adds a getGithubRoadmap query, and updates navigation constants.
Also updates the nav bar scrolling ( fixes #468 ) and the hero section orbit overflow behavior.
Contributor Checklist