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

stubbing out all the pages in the nav #20

Merged
merged 14 commits into from
Aug 11, 2020
Merged

stubbing out all the pages in the nav #20

merged 14 commits into from
Aug 11, 2020

Conversation

johnlindquist
Copy link
Contributor

@johnlindquist johnlindquist commented Aug 5, 2020

This is mainly to fix 404s in the nav.

I also played around with /lessons and /courses. I copy/pasted a tailwind card layout from here and filled them in with data from graphql. I imagine these will be deleted or refactored, but it was cool to see how far you could get with a tailwind component and graphql data.

I spent some time with types in courses and lessons, but back out those changes because that will be a bigger discussion.

@vercel
Copy link

vercel bot commented Aug 5, 2020

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

🔍 Inspect: https://vercel.com/eggheadio/egghead-io-nextjs/lmgudxs84
✅ Preview: https://egghead-io-nextjs-git-jl-fixing-404s.eggheadio1.vercel.app

export async function loadLessons() {
const query = /* GraphQL */ `
query getLessons {
lessons(per_page: 25) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a placeholder. I'd like to explore the pagination with that new Next.js hook you found.

Copy link
Contributor

Choose a reason for hiding this comment

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

We actively killed the /lessons route in favor of filtered search as a paginated list of 5000+ videos is...

https://twitter.com/chrisbiscardi/status/1172527924945096706?s=20

They capture it in this thread well.

)
}

export default function Courses({lessons}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export default function Courses({lessons}) {
export default function Lessons({lessons}) {

Copy link
Contributor

Choose a reason for hiding this comment

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

On egghead.io as it is now we dropped /lessons in favor of /search with a filter, and I think this is a good move. 5000+ lessons in a paginated list aren't very structurally predisposed to learning. 😂

)
}

export default function Courses({allCourses}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

/courses also forwards to /search on egghead.io for similar reasons as /lessons. The paginated list isn't very useful in practice.

I can see an argument for a curated /courses page with on-page filtered search.

export async function loadLessons() {
const query = /* GraphQL */ `
query getLessons {
lessons(per_page: 25) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We actively killed the /lessons route in favor of filtered search as a paginated list of 5000+ videos is...

https://twitter.com/chrisbiscardi/status/1172527924945096706?s=20

They capture it in this thread well.

@kodiakhq kodiakhq bot merged commit a164539 into main Aug 11, 2020
@kodiakhq kodiakhq bot deleted the jl/fixing-404s branch August 11, 2020 01:17
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