Skip to content
This repository was archived by the owner on Sep 25, 2025. It is now read-only.

feat(sponsors): add component for adding a sponsor to an event - #140

Merged
vaibhavsingh97 merged 21 commits into
freeCodeCamp:masterfrom
joelrozen:feat/add-sponsor
Nov 17, 2019
Merged

feat(sponsors): add component for adding a sponsor to an event#140
vaibhavsingh97 merged 21 commits into
freeCodeCamp:masterfrom
joelrozen:feat/add-sponsor

Conversation

@joelrozen

@joelrozenjoelrozen commented Oct 24, 2019

Copy link
Copy Markdown
Contributor

#97

  • I have read Chapter's contributing guidelines.
  • My pull request has a descriptive title (not a vague title like Update README.md).
  • My pull request targets the master branch of Chapter.

Closes#97

  • adds form to add a sponsor to an event.
  • display sponsor on event listing page.

@vkWeb - as discussed I started this before the change to TS and I don't know TS (yet!) :(

@vkWeb

Copy link
Copy Markdown
Member

@joelrozen Thanks for working on this. We really need to push MVP features into the codebase ❤️

Don't worry our mods will look at it and will point any issues in the code. I'm not into React as of now so I won't be able to review this.

I'm mentioning experienced devs for review: @nik-john@timmyichen@Zeko369.

@vkWeb
vkWeb requested a review from Zeko369October 24, 2019 07:04
Comment threadpackage.json Outdated
Comment threadclient/components/AddSponsor.tsx Outdated
Comment threadclient/components/AddSponsor.tsx Outdated
@vkWebvkWeb changed the title feat(add a sponsor): add component for adding a sponsor to an eventfeat(sponsors): add component for adding a sponsor to an eventOct 24, 2019
also make changes as per reviewer comments

@timmyichentimmyichen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work! Just added a few comments, mostly around TS and TODOs. Thanks for helping out with this 🙌 🎉

Comment threadclient/components/AddSponsor.tsx Outdated
Comment threadclient/components/AddSponsor.tsx
Comment threadpages/add-sponsor.tsx Outdated
Comment threadclient/components/AddSponsor.tsx Outdated
Comment threadclient/components/AddSponsor.tsx
Comment threadclient/components/AddSponsor.tsx Outdated
Comment threadclient/components/AddSponsor.tsx Outdated
Comment threadclient/components/AddSponsor.tsx Outdated
Comment threadpages/add-sponsor.tsx Outdated
@timmyichen

Copy link
Copy Markdown
Contributor

Btw let me know if you need any help with TS, either here or on Discord 🙃

@timmyichen

Copy link
Copy Markdown
Contributor

Oops sorry for the string of comments, but can you also revert the changes to package-lock.json since axios is no longer required? Thanks 😛

@joelrozen

Copy link
Copy Markdown
ContributorAuthor

Oops sorry for the string of comments, but can you also revert the changes to package-lock.json since axios is no longer required? Thanks 😛

Do you want me to just add package-lock.json and yarn.lock to .gitignore?

@vaibhavsingh97

Copy link
Copy Markdown
Member

Do you want me to just add package-lock.json and yarn.lock to .gitignore?

Nope, it's needed in the project and it's recommended to present in git

@nik-john

Copy link
Copy Markdown
Contributor

We should be using either yarn or npm, there's no need the use both. I would recommend just using npm now that it's pretty much caught up with yarn

Comment threadclient/components/AddSponsor.tsx Outdated
Comment threadpages/add-sponsor.tsx Outdated
Comment threadclient/components/AddSponsor.tsx
Comment threadclient/components/AddSponsor.tsx Outdated
Comment threadclient/components/AddSponsor.tsx Outdated
Comment threadclient/components/AddSponsor.tsx Outdated

@vaibhavsingh97vaibhavsingh97 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, just a minor thing, move AddSponsor.tsx to Components folder in interfaces and styles.
FYR:

|
|__interfaces
| |__Components
| |_AddSponsor.tsx
|__styles
| |__Components
| |_AddSponsor.tsx

@joelrozen

Copy link
Copy Markdown
ContributorAuthor

LGTM, just a minor thing, move AddSponsor.tsx to Components folder in interfaces and styles.
FYR:

|
|__interfaces
| |__Components
| |_AddSponsor.tsx
|__styles
| |__Components
| |_AddSponsor.tsx

done!

@vaibhavsingh97vaibhavsingh97 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM 🎉, will wait for another approval before the merge

@timmyichentimmyichen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sweet! Just a few last changes and we should be good to go

Comment threadclient/components/AddSponsor.tsx Outdated
Comment threadclient/components/AddSponsor.tsx Outdated
Comment threadclient/components/AddSponsor.tsx Outdated

@timmyichentimmyichen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Zeko369Zeko369 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment threadclient/components/AddSponsor.tsx Outdated
Comment threadclient/interfaces/components/AddSponsor.tsx Outdated
Comment threadclient/components/AddSponsor.tsx
@QuincyLarson

Copy link
Copy Markdown
Contributor

Note: Per #163 we have decided to use React-hook-form.

@joelrozen

Copy link
Copy Markdown
ContributorAuthor

I will refactor tomorrow

@joelrozen

Copy link
Copy Markdown
ContributorAuthor

@timmyichen and @QuincyLarson changes complete. Also now makes (basic) use of react-hooks-form as per #163

@timmyichentimmyichen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! cc @Zeko369

@vaibhavsingh97vaibhavsingh97 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, just a minor change

Comment threadclient/components/AddSponsor.tsx Outdated

@vaibhavsingh97vaibhavsingh97 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM 🎉

@vaibhavsingh97

Copy link
Copy Markdown
Member

Merging, Thanks for the PR @joelrozen 💯

@vaibhavsingh97
vaibhavsingh97 merged commit 8bcfd3b into freeCodeCamp:masterNov 17, 2019
@phoenisx

Copy link
Copy Markdown
Contributor

So are we going with isomorphic-fetch then?? It hasn't been maintained for 4 years now
Ref: matthew-andrews/isomorphic-fetch#133

@vaibhavsingh97

Copy link
Copy Markdown
Member

@Shub1427 that's not the case here, please feel free to raise another PR if you want to use axios. This PR is there for long time, so i thought let's merge this and then work actively on fixing issues with PR.

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

As an organizer I can add a venue sponsor to the event with a link to their website as a way of thanking them for hosting.

9 participants

@joelrozen@vkWeb@timmyichen@vaibhavsingh97@nik-john@QuincyLarson@phoenisx@Zeko369@robertt