BuidlGuidl / grants.buidlguidl.com

https://grants-bg.vercel.app
MIT License
3 stars 3 forks source link

Show the number of previous grants submitted for a user #107

Closed Pabl0cks closed 7 months ago

Pabl0cks commented 7 months ago

Fixes #103

image

Some notes to check:

Error: React Hook "useSWR" is called conditionally. React Hooks must be called in the exact same order in every component render.

vercel[bot] commented 7 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
grants-bg ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 15, 2024 9:46am
damianmarti commented 7 months ago

Fixes #103

image

Some notes to check:

  • Not sure if code comments are too obvious, lmk to delete 🙌
  • Should we name it "# submissions" or "# previous submissions"
  • I was getting the error below, so chatGPT recommended to move the useSWR code up there (before the if (grant.status !== PROPOSAL_STATUS.PROPOSED ...

Error: React Hook "useSWR" is called conditionally. React Hooks must be called in the exact same order in every component render.

Yes, it's because you can't add a hook with a condition, because you always need to have the same hooks in the same order (this is an internal limitation from React hooks)

The code looks good to me (but I'm not familiar with Grant code, so maybe I'm missing something).

One little comment. The issue said to add the status of the submission on hovering, but maybe it is more useful (and it seems to have space) to show that information with no hovering, so it's easy to show all the information without having to do the hovering (something like 3 submissions (2 approved, 1 completed)).

Pabl0cks commented 7 months ago

Thanks for the review and explanation @damianmarti !! ♥

One little comment. The issue said to add the status of the submission on hovering, but maybe it is more useful (and it seems to have space) to show that information with no hovering, so it's easy to show all the information without having to do the hovering (something like 3 submissions (2 approved, 1 completed)).

Yeah! I think we could show it right now, but the window started with very little info and we're adding a lot more stuff after admins feedback, so probably it's good to save some space here! 🙌

Pabl0cks commented 7 months ago

Looks graet @Pabl0cks !! added a couple of nitpick please feel free to ignore but this is already ready to merge 🙌 !!

Thanks for the review @technophile-04!! ♥

I think my brain was dead when I did that 😅, should always check if there is a daisyUI solution for my problem, but in this case I had already several tooltips in this page to look!! My fault, sorry!

carletex commented 7 months ago

testing and reviewing!