-
Notifications
You must be signed in to change notification settings - Fork 0
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
Team member integration with Contentful. #92
base: develop
Are you sure you want to change the base?
Conversation
…on the event card.
…e from Contentful instead of eamMemberData.json.
✅ Deploy Preview for sfusurgedev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
We should avoid client-side fetching for contentful, as it will significantly drive up the amount of requests to our contentful account for every unique visitor.
Better to restructure the code so that the data is fetched from a server component, then passed to a client component. Ensure that the request is also cached to reduce load on the server for contentful fetches.
Additional requested changes are noted in the comments. Please address and then resubmit PR once resolved
Thanks 🫡
src/app/api/team/route.ts
Outdated
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 unnecessary, the request should be made using the contentfulClient through a server component, not on the client side.
src/components/EventCard.tsx
Outdated
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.
The EventCardProps should be using the defined EventDTO, as the purpose of a Data Transfer Object is to serialize the contentful data types to native typescript types for use in the client.
Not sure the purpose of the Image type either, is it mapped to a content type in contentful? It's easier to just get the CDN image url from the contentful request and use it directly in the image src property
…and stored the team member data on the user's local cache. Other than that, generally cleaned up code.
Describe your changes
Completed team member integration in Contentful. Now Team Members come from Contentful instead of teamMemberData.json.
Describe your changes
Implemented getting the preview image from Contentful and putting it on the event card.
Testing steps
content-updates
pnpm install
to update dependenciesIssue ticket number and link
#91