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

[ALT-1508] feat: add gatsby project to templates and add readmes for templates and test-apps #896

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

ryunsong-contentful
Copy link
Contributor

@ryunsong-contentful ryunsong-contentful commented Jan 6, 2025

Purpose

Add gatsby to the examples directory so that it can be used for the Gatsby dev doc guide.

There's two examples, one for SPA client side Gatsby apps and the other for SSG. The SSG has an additional file gatsby-node.js to polyfill the path import in order to run the project locally.

Approach

Started a new gatsby project by doing the following steps

  1. npm init gatsby

Integrate with Experiences

  1. npm i @contentful/experiences-sdk-react
  2. Created examples/gatsby/src/utils/useContentfulClient.js file to create the Contentful Client to hit the Contentful CDN
  3. Created examples/gatsby/src/pages/experiences-slug-example.js which uses the Client to fetch the experience object and use the to render the Experience.

@ryunsong-contentful ryunsong-contentful requested review from a team as code owners January 6, 2025 23:52
Copy link

vercel bot commented Jan 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nextjs-marketing-demo-bug-test ✅ Ready (Inspect) Visit Preview Jan 9, 2025 10:34pm
3 Skipped Deployments
Name Status Preview Updated (UTC)
experience-builder-test-app ⬜️ Ignored (Inspect) Jan 9, 2025 10:34pm
studio-nextjs-marketing-demo ⬜️ Ignored (Inspect) Jan 9, 2025 10:34pm
studio-react-vite-template ⬜️ Ignored (Inspect) Jan 9, 2025 10:34pm

@ryunsong-contentful ryunsong-contentful force-pushed the ALT-1508 branch 3 times, most recently from 89fc17b to f69a7b5 Compare January 6, 2025 23:53
@ryunsong-contentful ryunsong-contentful force-pushed the ALT-1508 branch 2 times, most recently from ee40a1b to 6e4d174 Compare January 7, 2025 17:45
Copy link
Contributor

@primeinteger primeinteger left a comment

Choose a reason for hiding this comment

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

I wonder if the file structure should be flattened to be consistent with the next apps:

  • examples/gatsby-spa
  • examples/gatsby-ssg

Copy link
Contributor

@primeinteger primeinteger left a comment

Choose a reason for hiding this comment

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

Just a few comments so far before I start testing. Thanks for taking more time to switch it to TS and making it more consistent with other example apps! I only reviewed the SPA app but my comments could apply to the SSG app too

examples/gatsby/gatsby-spa/README.md Outdated Show resolved Hide resolved
examples/gatsby/gatsby-spa/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@primeinteger primeinteger Jan 9, 2025

Choose a reason for hiding this comment

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

In my opinion, we don't need all this css and html content in our example app. As an alternative, this file could just render a <h1> tag that says "Studio Experiences with Gatsby single page application example" or something else very minimal like that

Copy link
Member

Choose a reason for hiding this comment

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

The other example apps redirect the '/' route to '/home-page', and the routing is setup so the segment after the root is the slug that gets passed to fetch the experience.

Comment on lines 5 to 7
const slug = ''
const localeCode = '';
const experienceTypeId = '';
Copy link
Contributor

Choose a reason for hiding this comment

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

I find it a little bit odd to hardcode the slug and locale to one value. Is that intentional because of being a single-page application?

We should use the GATSBY_CTFL_EXPERIENCE_TYPE environment variable for the experienceTypeId

Copy link
Member

Choose a reason for hiding this comment

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

Ya, I think the slug should be dynamic based on the route (see my previous comment).

For the locale, the other example apps are using built in i18n to detect a localeCode. Does gatsby have anything similar that we could use for providing the locale?

Copy link
Contributor Author

@ryunsong-contentful ryunsong-contentful Jan 9, 2025

Choose a reason for hiding this comment

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

It doesn't have anything for i18n like Next does. This probably falls under the nice to have category but ultimately falls under the responsibility of the end user to figure out. Similar to how if they want to integrate with tailwind, that's on them and not on us to show how to do.

Maybe some exceptions in the future will be when we have full-fledged tutorials and then point to an example that has bells and whistles.

But maybe we can do what you did with astro and add some config file that holds these values and can reference that instead.

Comment on lines +33 to +28
3. **Create the Experience in the Contentful Experienecs UI**

In the settings tab of Experiences, ensure that your Slug field matches the slug in step 1. Save your changes and publish to see this Gatsby project hooked up with Contentful Experiences.
Copy link
Contributor

@primeinteger primeinteger Jan 9, 2025

Choose a reason for hiding this comment

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

These steps may need to mention what Content Preview URL to configure. I can't figure it out. I tried:

  • http://localhost:8000 -> shows the gatsby index page (not studio-enabled)
  • http://localhost:8000/{entry.fields.slug} -> shows the 404 page (even though I hardcoded the slug variable in ExperienceSlugExample.tsx to match the slug of my experience and published it)
    Screenshot 2025-01-09 at 9 29 18 AM
    Screenshot 2025-01-09 at 9 28 57 AM

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 updated the README to make it work properly

Comment on lines +28 to +31
<main style={pageStyles}>
<h1 style={headingStyles}>Page not found</h1>
<p style={paragraphStyles}>
Sorry 😔, we couldn’t find what you were looking for.
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed (as I'm struggling to get the example running) that I'm seeing a 404 page but the content doesn't match this file. Any idea where the 404 content below comes from?

Screenshot 2025-01-09 at 9 28 57 AM

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 updated the README to make it work properly

import { fetchById, detachExperienceStyles } from '@contentful/experiences-sdk-react';
import { useContentfulClient } from '../utils/useContentfulClient';

exports.createPages = async ({ graphql, actions, reporter }: any) => {
Copy link
Member

Choose a reason for hiding this comment

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

I get some errors when trying to run npm run build:

ERROR #11328  API.NODE.VALIDATION

The plugin "gatsby-plugin-page-creator" created a page without a valid default export.

This should create a static site with all those experiences prebuilt.

Ideally, this would create the experiences to follow the same routing structure as the other apps (www.mysite.com/{slug}) and create a page for each experience that is in the space being used.

@ryunsong-contentful ryunsong-contentful force-pushed the ALT-1508 branch 2 times, most recently from c9e19be to d64b984 Compare January 9, 2025 22:24
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