-
-
Notifications
You must be signed in to change notification settings - Fork 211
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
Enhancement: Roadmap page for the Overview section #1021
Conversation
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.
Welcome to the JSON Schema Community. Thanks a lot for creating your first pull request!! 🎉🎉 We are so excited you are here! We hope this is only the first of many! For more details check out README.md file.
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1021 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 10
Lines 373 373
Branches 94 94
=========================================
Hits 373 373 ☔ View full report in Codecov by Sentry. |
Hi @rishabhknowss. is this ready for review? It seems uncompleted. One thing to consider is remember that we have dark/light theme selection so you need to implement that behavior as well. I am saying this because it seems that the current version in this PR was created only for dark theme. In addition, the roadmap page will be into the overview section of the docs, so my advice is to place this feature into the /pages/overview/roadmap folder, and use the same tsx approach than other overview pages like case studies or use cases. |
ohh, totally forgot about the themes....making changes |
Please, check the correct colors in all labels and styles of each card because in both dark and light themes. |
I have added both the themes , fixed the route , adjusted the labels according to the board and made it responsive Screencast.from.2024-10-13.15-20-01.webm |
@benjagm I think we may want to consider revisiting the roadmap given our changed circumstances. Have our goals or priorities changed since we're not working on this full time anymore? Probably a good idea to figure this out before we post it on the website. |
This is a great point. The great part of this approach is the data is coming from this board: https://github.com/orgs/json-schema-org/projects/14 so it will dynamically show live data from GitHub. While the roadmap discussion is in progress we can hide the table and show a different message explaining that the roadmap discussion is happening and inviting everyone to be part of it. |
yup guys it will reflect the changes made on the roadmap board |
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.
Are you sure this is the final version of the code? I am missing most of the UX you shared in this screenshot:
This is what I see in this version:
In addition, the roadmap page will be part of the sidebar in the overview section of our docs. You should use a different Typescript page with sidebar capabilities. Please use the same approach used for other pages inside the overview section:
https://github.com/json-schema-org/website/blob/main/pages/overview/use-cases/index.page.tsx
or:
https://github.com/json-schema-org/website/blob/main/pages/overview/case-studies/index.page.tsx
okay resolving the issue |
Hi @benjagm , thanks for the follow-up. I think the code is now ready for review. I have changed the design a little bit and followed the same approach as other pages have. |
I think there's still a deployment issue, as I can also see no UX. However, it is working perfectly on local. |
i have implemented something , let me see if it works |
It seems that Tailwind’s PurgeCSS, because of the theme definition, is affecting the styles you are trying to use. To avoid this, you can add a "safelist" in the
|
Thanks @benjagm , its working now |
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.
This is looking great and we just need to make some UX adjustments:
* Those gradients remind me too much AsyncAPI's branding. Can we change it to use our cooperative blue as base color instead of that violet?-
We need some separation between the circle attached to each card and the vertical line.
-
Let's remove the names of the assigned contributors.
-
Let's define consistent colors for all the labels:
Effort: Red for very high colors, to yellow to medium values to green to low values
Impact: Green for very high colors, to yellow to medium values to red to low values
Status: Green in progress, violet completed, red paused, blue deferred and planned
I hope this helps.
making changes |
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.
We are getting very close!!
After reviewing the changes we made after my suggestion I realized that there is a better approach by following best practices to avoid Tailwind CSS purge. Check my comment on that. We are likely going to remove all those changes in the tailwind conf by making sure we don't concatenate string to create class names. https://v2.tailwindcss.com/docs/optimizing-for-production#writing-purgeable-html
In addition, for the mobile version it seems we have bigger left padding and it will be better if we make sure the content is centered with same x padding.
Great job! We are one step closer
making changes :> |
Co-authored-by: Benjamin Granados <[email protected]>
Co-authored-by: Benjamin Granados <[email protected]>
i think its done |
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.
Amazing job! This is ready to merge!
Congratulations, @rishabhknowss for your first pull request merge in this repository! 🎉🎉. Thanks for your contribution to JSON Schema! |
What kind of change does this PR introduce?
Adds a Roadmap page
Issue Number:
Screenshots/videos: