Skip to content
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

feat: add repositories section #30

Merged
merged 11 commits into from
Nov 28, 2024
Merged

Conversation

vfshera
Copy link
Collaborator

@vfshera vfshera commented Nov 22, 2024

  • Develop the repositories section on the landing page per the Figma designs
  • Make sure the components are fully responsive
  • Write Unit and e2e tests in the tests folder, to test the components' look, feel, and functionality
  • Include a list of any API endpoints required in the pull request description, if applicable

@vfshera
Copy link
Collaborator Author

vfshera commented Nov 22, 2024

On the third task I saw it is in every issue and currently not setup. I created 2 issues #28 and #29 , so that it may be setup first before every contributor creates as specific test

package.json Outdated
@@ -62,6 +62,7 @@
"react-to-print": "^2.15.1",
"recharts": "^2.12.7",
"sonner": "^1.5.0",
"swiper": "^11.1.15",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice if tried to avoid extra dependancies until absolutely necessary

}, [projects, searchTerm, selectedLanguage]);

return (
<section className="border-y border-[#294740] px-5 pb-16 pt-5">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's try and centralize all the colors in our tailwind config and only use tokens

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

giving the section w-full will help it be responsive on mobile , also abstanin from positioning using padding preffer using flexbox or gird


<div className="mt-[30px] grid gap-x-8 gap-y-[30px] md:grid-cols-2 lg:grid-cols-3">
{searchedProjects.map((project) => (
<div className="relative isolate overflow-hidden rounded-[12px] border border-[#294740]/80 bg-[#413535]/[.53] px-4 pb-5 pt-7">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing list key

<div className="mt-[30px] grid gap-x-8 gap-y-[30px] md:grid-cols-2 lg:grid-cols-3">
{searchedProjects.map((project) => (
<div className="relative isolate overflow-hidden rounded-[12px] border border-[#294740]/80 bg-[#413535]/[.53] px-4 pb-5 pt-7">
<svg
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's try and centralize all the inlined svg ocons inside the src/components/wrappers/custom-icons.tsx file

</div>

<div className="mt-5">
<Swiper
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if possible explain or show a demo of what the Swipper dependancy will help achieve

@tigawanna
Copy link
Collaborator

looks better but still a few nitpicks

  • the tags aren't overflowing properly on mobile
    image
  • running the build command is failing because of type error issues with the swiper package , cnsider implementing without it and use shadcn cards instead until it's hard to do with css animations

@tigawanna tigawanna merged commit c970258 into main Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants