Skip to content
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

Pull sanity data for curated instructor pages #602

Merged
merged 14 commits into from
May 4, 2021

Conversation

zacjones93
Copy link
Contributor

All of our curated instructor data is currently hard coded on the front end.

This loads instructor data from sanity and passes it into curated instructor pages as sanityInstructor

One note, the linter was going ham on all the files so I committed with no-verify. Not sure what would have changed there but would like someone to run this on their machine.

Learn web development from Stephanie Eckles on egghead  egghead io
Learn web development from Stephanie Eckles on egghead  egghead io

@vercel
Copy link

vercel bot commented Apr 28, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

egghead-next-storybook – ./

🔍 Inspect: https://vercel.com/eggheadio/egghead-next-storybook/5dNQhpbxRvPvE1ot9y2MRDfN8zC5
✅ Preview: https://egghead-next-s-git-zj-update-stephanie-eckles-landing-pa-dff5b0.vercel.app

egghead-io-nextjs – ./

🔍 Inspect: https://vercel.com/eggheadio/egghead-io-nextjs/6kou3xL2pFJtvSMwTuZBECJWEbCB
✅ Preview: https://egghead-io-nex-git-zj-update-stephanie-eckles-landing-pa-08cc7c.vercel.app

Comment on lines 196 to 199
<InstructorCuratedPage
instructor={instructor}
sanityInstructor={sanityInstructor}
/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pass sanity data down

@@ -14,6 +15,7 @@ const InstructorsIndex: any = {
'flavio-corpa': SearchFlavioCorpa,
'hiro-nishimura': SearchHirokoNishimura,
'chris-biscardi': SearchChrisBiscardi,
'stephanie-eckles': SearchStephanieEckles,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

add stephanie to the instructors index for curated pages

Comment on lines 38 to 55
<SearchInstructorEssential
instructor={combinedInstructor}
CTAComponent={
<CssFormStyling
resource={primaryCourse}
location="Stephanie Eckles instructor page"
/>
}
/>
<section className="grid grid-cols-4 gap-3 -mt-10 mb-10 pb-10 xl:px-0 px-5 max-w-screen-xl mx-auto dark:bg-gray-900 w-full">
<ProjectStack className="col-span-1" data={projects.resources} />
<div className="col-span-3 grid grid-cols-2 gap-3">
<CardHorizontal className="col-span-2" resource={secondCourse} />
<CardHorizontal className="col-span-1" resource={thirdCourse} />
<CardHorizontal className="col-span-1" resource={fourthCourse} />
</div>
</section>
</div>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

display courses and projects on page

Comment on lines +59 to +81
export const stephanieEcklesQuery = groq`*[_type == 'resource' && slug.current == "stephanie-eckles-landing-page"][0]{
'projects': resources[slug.current == 'instructor-landing-page-projects'][0]{
resources[]{
title,
'path': url,
description,
image
}
},
'courses': resources[slug.current == 'instructor-landing-page-featured-courses'][0]{
resources[]->{
title,
'description': summary,
path,
byline,
image,
'background': images[label == 'feature-card-background'][0].url,
'instructor': collaborators[]->[role == 'instructor'][0]{
'name': person->.name
},
}
},
}`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

sanity query

Comment on lines +42 to +64

const sanityInstructorHash = {
'stephanie-eckles': stephanieEcklesQuery,
}

type SelectedInstructor = keyof typeof sanityInstructorHash

const canLoadSanityInstructor = (
selectedInstructor: string,
): selectedInstructor is SelectedInstructor => {
const keyNames = Object.keys(sanityInstructorHash)

return keyNames.includes(selectedInstructor)
}

export const loadSanityInstructor = async (selectedInstructor: string) => {
if (!canLoadSanityInstructor(selectedInstructor)) return

const query = sanityInstructorHash[selectedInstructor]
if (!query) return

return await sanityClient.fetch(query)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@theianjones helped me implement this.

loadSanityInstructor uses a type guard (canLoadSanityInstructor) to check that the instructor slug (the one that will be passed in via getServerSideProps) is present in the defined sanityInstructorHash as a key. If it is, that key is associated with the query that it will call.

This makes sure that we are only querying for instructor data on the pages that we want to load sanity data onto.

Comment on lines 216 to 220
try {
initialInstructor = await loadInstructor(instructorSlug)

sanityInstructor = await loadSanityInstructor(instructorSlug)
} catch (error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When an instructor filter is selected we will load sanity data (if it passes the type guard)

@@ -34,6 +34,7 @@ type SearchProps = {
searchClient?: any
searchState?: any
instructor?: any
Copy link
Contributor

Choose a reason for hiding this comment

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

this stands out to me. I do t like “sanity” as special data. could we merge this into a si file “instructor” object so it’s “just an instructor” and then display data based on the shape of instructor and the available properties

instructor: any
sanityInstructor: any
}) {
const combinedInstructor = {...instructor}
Copy link
Contributor

Choose a reason for hiding this comment

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

this could actually be combined at a higher level

@@ -211,6 +215,8 @@ export const getServerSideProps: GetServerSideProps = async function ({
)
try {
initialInstructor = await loadInstructor(instructorSlug)

Copy link
Contributor

Choose a reason for hiding this comment

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

here’s where I’d consider a single “load instructor” that loads from all sources

Comment on lines +216 to +221
sanityInstructor = await loadSanityInstructor(instructorSlug)

initialInstructor = {
...initialInstructor,
...sanityInstructor,
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

None of the data is being overridden but it makes sense to me that sanity data would take precedence over the initialInstructor data

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this pattern makes a lot of sense so we don't have two+ data sources at the component, we just bring it together at the server level.

Copy link
Contributor

Choose a reason for hiding this comment

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

since we are using a stale-while-revalidate cache header, it should be performant!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, it was also super easy to refactor

Comment on lines 43 to 51
<section className="grid grid-cols-4 gap-3 -mt-10 mb-10 pb-10 xl:px-0 px-5 max-w-screen-xl mx-auto dark:bg-gray-900 w-full">
<ProjectStack className="col-span-1" data={projects.resources} />
<div className="col-span-3 grid grid-cols-2 gap-3">
<CardHorizontal className="col-span-2" resource={secondCourse} />
<CardHorizontal className="col-span-1" resource={thirdCourse} />
<CardHorizontal className="col-span-1" resource={fourthCourse} />
</div>
</section>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

This layout needs work at all breakpoints. Using the CardHorizontal component doesn't seem to be the right approach for this layout.

CleanShot 2021-04-29 at 12 54 13@2x

</div>
</a>
</Link>
)}
Copy link
Contributor

Choose a reason for hiding this comment

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

The description is awkward and lacks legibility. I would consider exploring expanding the project section or a smaller font.

image

@@ -36,3 +39,26 @@ export async function loadInstructor(slug: string) {

return instructor
}

const sanityInstructorHash = {
'stephanie-eckles': stephanieEcklesQuery,
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this pattern!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants