Rebuild-Black-Business / RBB-Website

Website to help connect black-owned businesses with consumers and resources
https://www.rebuildblackbusiness.com/
MIT License
119 stars 75 forks source link

Feature/create fundraisers page #321

Closed Mhmdabed11 closed 4 years ago

Mhmdabed11 commented 4 years ago

Describe your PR

Create the fundraisers page Related to #289

Pages/Interfaces that will change

A new fundraisers page will be created

Screenshots / video of changes

Steps to test

  1. Navigate to /fundraisers
netlify[bot] commented 4 years ago

Deploy request for rebuild-black-business accepted.

Accepted with commit 8cb20f629765d16136ffc5feb151bda3a59d8bff

https://app.netlify.com/sites/rebuild-black-business/deploys/5f43fd1b596d890008fdde5b

Mhmdabed11 commented 4 years ago

@magnificode I have some questions regarding this PR so it can be 100% done from my side:

  1. Is there a way to filter the businesses which have a donationLink entry using the REST API. I cannot fins such one in the api docs.
  2. image For the above image, what should happen when we press on the button? And also I need the public ID of the background image if possible. 3.For this page, I think it makes sense to have more than 12 entries in the grid since the size of each gridItem is relatively small. This means that we need to edit the useSearch hook to accept an optional pageSize argument. I can do this if we agree to do it.
magnificode commented 4 years ago

@magnificode I have some questions regarding this PR so it can be 100% done from my side:

  1. Is there a way to filter the businesses which have a donationLink entry using the REST API. I cannot fins such one in the api docs.
  2. image For the above image, what should happen when we press on the button? And also I need the public ID of the background image if possible. 3.For this page, I think it makes sense to have more than 12 entries in the grid since the size of each gridItem is relatively small. This means that we need to edit the useSearch hook to accept an optional pageSize argument. I can do this if we agree to do it.

Excellent questions @Mhmdabed11

  1. We should be able to, maybe @elchris can shed some light there?
  2. Public ID for that image: coffee-shop_rbgwyx - @shapirone, could you speak to the functionality for that CTA?
  3. It looks like the designs have 14 items output on this page. I think adding that optional argument would be excellent!

Last thing, would you mind removing the "Home" link from the nav? The additional fundraisers page makes the nav a bit long.

Screen Shot 2020-08-20 at 11 31 32 AM

Based on the design, the order should be as follows:

Screen Shot 2020-08-20 at 11 40 15 AM

Thank you so much for the hard work on this @Mhmdabed11 - this is looking great!

Mhmdabed11 commented 4 years ago

@magnificode Thank you for answering my questions and you are most welcome ! I will work on the issues discussed above :+1:

elchris commented 4 years ago

oh my bad to get businesses raising funds, pass:

in-need=true

It's been documented on the API Doc:

https://developer.rbb-api.com/doc

click on the 3rd endpoint:

GET /api/v1/businesses

@Mhmdabed11 @magnificode

Mhmdabed11 commented 4 years ago

@elchris thank you for your reply. I actually did read this in the docs, but I ended up with some businesses with donationLink:null while setting in-need=true. So the donate button will not take the user to anywhere.

elchris commented 4 years ago

@Mhmdabed11 Oh i see, i understand now, thank you for the clarification.

Some time later tonight i'll add a hasDonationLink=true filter to the API. It will work independently from "in-need", which you won't need to specify. With this said, it's okay if you stay with your current approach directly to AirTable. But I will let you know when the new parameter is ready, just in case you can switch back to the API.

Thank you for your help and patience!

cc: @magnificode

Mhmdabed11 commented 4 years ago

@elchris Thank you for your help. Actually I have switched to using the API already, so it is fine I will wait.

elchris commented 4 years ago

@Mhmdabed11 okay i've just deployed an update. You should be able to:

And if you don't pass the parameter, it will keep the current functionality.

shapirone commented 4 years ago

@Mhmdabed11 the CTA is supposed to trigger the form for adding a fundraiser

Mhmdabed11 commented 4 years ago

@Mhmdabed11 the CTA is supposed to trigger the form for adding a fundraiser

For this, I added an isFundraiser prop to the BusinessSignupForm component which is false by default. If we are trying to add a fundraiser, then the user must enter a Donation Link in the form and he cannot submit it without entering one. Please let me know if you agree with me on this. I am also happy to discuss further.

cc: @magnificode

shapirone commented 4 years ago

@Mhmdabed11 sorry I wasn't clear but since we support fundraisers that aren't directly attached to businesses, we have a different (and much simpler) UX for adding a fundraisers vs businesses. Designs can be found in our Figma.

@magnificode do we have a separate issue for creating the Fundraiser form or should that be handled as part of this ticket?

magnificode commented 4 years ago

@shapirone @Mhmdabed11 lets focus on displaying the existing businesses with donation links in this PR.

We'll create another task for submitting a fundraiser not connected to a business.

magnificode commented 4 years ago

@Mhmdabed11 this looks awesome, and great touch pre-checking the "Has Fundraiser" checkbox when submitting from this page. I think that this will work great for now, we can iterate on the data structure on the backend further down the line if we want to split this out into it's own table.

The final, tiny thing, if you wouldn't mind @Mhmdabed11 - could we add a couple of transforms to this image? As it stands, the image itself makes the text on top a bit tough to read.

Screen Shot 2020-08-24 at 10 39 23 AM

In order to match the design, I added a couple of transformations to the image on cloudinary:

background: "#000000"
effect: "saturation:-78"
opacity: "47"

The Cloudinary Image takes a transformations object as props, which you should be able to pass these values to, in order to see the result, which should look similar to this: https://res.cloudinary.com/rebuild-black-business/image/upload/b_rgb:000000,e_saturation:-78,o_47/v1597945031/assets/coffee-shop_rbgwyx.jpg

Let me know if you're able to get that to work!

Thank you again for all the hard work! Once that image is handled we can get this merged in :)

magnificode commented 4 years ago

Awesome work @Mhmdabed11 merging this in now!