-
Notifications
You must be signed in to change notification settings - Fork 75
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
Custom forms #297
Custom forms #297
Conversation
Deploy request for rebuild-black-business accepted. Accepted with commit 7e04277 https://app.netlify.com/sites/rebuild-black-business/deploys/5eff6130ab6404000854f7ae |
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 great @SeHarlan thank you so much for taking this on. It'll really help set us up to be able to handle the data in a much better way!
import PrimaryButton from '../Buttons/PrimaryButton'; | ||
import { submitAlly } from '../../services/AirtableServices'; | ||
|
||
const skillTypes = [ |
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.
@SeHarlan are we able to pull the skill types from Airtable via the API? Just thinking of ways we can possibly keep things as dynamic as possible!
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 proving really difficult. I've looked through all the docs and as far as I can see they don't provide access to the select options. There are quite a few complaints about it in the community forums lol. I made a custom hook that grabs the first page of docs and filters for just the specialty field and then I reduce it to only unique items. But this really isn't performant. Not sure what you wanna do with it so I'll leave it in just in case.
the dynamic fetch wasn't catching the businesses "Other" category so I add it in conditionally on line 88 on AirtableServices.js |
Exports in components/index.js were left as airtable embeds cause I don't know everything those might be linked to and didn't wanna mess up any fallbacks that might exist. |
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.
With the change of adding GATSBY_
prefix to the airtable environment variables, the gatsby-config.js
file will also need updated along with .env.EXAMPLE
and README.md
. After updating the gatsby-config I was able to get passing tests locally (an admin will need to update Circle CI after the code change to get a ✔️ to show here)
@SeHarlan This has been merged, can't thank you enough for the hard work here! This will help us to be able to move on some of our tasks to help improve the UX around business submissions! |
Describe your PR
Related to #295
Fixes #
Pages/Interfaces that will change
businesses.js - commented out Airtable embed added Custome form and added (closeOnOverlayClick={false}) to parent modal for better functionality (like a success message)
AllyFeed.js - same as above except with AllySignUpForm
Screenshots / video of changes
Steps to test
GATSBY_AIRTABLE_API_KEY=
GATSBY_AIRTABLE_BASE_ID=
Additional notes