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

Static preview #2295

Merged
merged 19 commits into from
Dec 4, 2024
Merged

Static preview #2295

merged 19 commits into from
Dec 4, 2024

Conversation

pwizla
Copy link
Collaborator

@pwizla pwizla commented Nov 22, 2024

This PR adds docs for Static Preview (it targets the beta version, so no mention of feature flags):

Copy link

vercel bot commented Nov 22, 2024

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

Name Status Preview Comments Updated (UTC)
documentation ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 4, 2024 9:28am

@pwizla pwizla requested review from remidej and Marc-Roig November 22, 2024 14:03
@pwizla pwizla self-assigned this Nov 22, 2024
@pwizla pwizla added pr: new content PRs for new product features or new documentation sections source: Dev Docs PRs/issues targeting the Developer Docs source: User Guide PRs/issues targeting the User Guide labels Nov 22, 2024
@pwizla pwizla added this to the 5.3.0 milestone Nov 22, 2024
@remidej
Copy link
Contributor

remidej commented Nov 22, 2024

This is really good!

2 things about the next.js guide part:

  • with the code you suggest they'll run into the issue where they will never exit Next.js draft mode after opening it. This is the bug you encountered yesterday where you see draft content even in the published tab. You can see how I fixed in in the launchpad. Basically you need to always go through the next.js preview route, so that you can force the draft mode cookie removal when the status is published.
  • since your guide is pretty in depth already, I would add the last missing piece, which is to check if draft mode is enabled in the places that are querying Strapi data, so they can know whether they should add the status=draft param. See this part of the launchpad code for an example.

@yanniskadiri
Copy link
Contributor

yanniskadiri commented Nov 25, 2024

I just checked the user guide part, it looks good to me. This is great work, thanks @pwizla!

@pwizla
Copy link
Collaborator Author

pwizla commented Nov 29, 2024

@remidej I did my best to improve the implementation guide based on your feedback 😊 What do you think?

Copy link
Contributor

@remidej remidej left a comment

Choose a reason for hiding this comment

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

thanks for the updates!

docusaurus/docs/dev-docs/preview.md Outdated Show resolved Hide resolved
docusaurus/docs/dev-docs/preview.md Outdated Show resolved Hide resolved
1. Create or adapt your data fetching utility to check if draft mode is enabled
2. Update your API calls to include the draft status parameter when appropriate

The following, taken from the [Launchpad](https://github.com/strapi/LaunchPad/tree/feat/preview) Strapi demo application, is an example of how to implement draft-aware data fetching in your Next.js front-end application:
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that these links are pointing to the feat/preview branch of the launchpad repo, which will 404 when we merge our PR on the main branch. Hopefully next week after the CMS release

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the reminder! Yes, I'm used to it, and still haven't found a satisfying solution to automate this in some way. Any idea? 🤔 Or maybe is there another format of URLs that would persist across merges? 🤔
I'll update it once the strapi/strapi branch is merged.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you could link to the repo's tree for a specific commit, this will never 404: https://github.com/strapi/LaunchPad/tree/faa960bf8ca824f5b92dc984c4627abeb87b85eb

But the downside is you wouldn't get any potential improvement that comes in the future. Maybe with a self reminder to update the commit once in a while it could work?

docusaurus/docs/dev-docs/preview.md Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm realizing now there's one last step needed to get the whole thing to work. Apologies for not seeing this earlier.

The permissions to load an iframe go both ways:

  • the parent window needs to allow loading the child window (that we do, it's the allowedOrigins thing)
  • the child window (the preview site) needs to allow being embedded in the parent (the admin)

That second step requires the preview frontend to have its own header directive: the CSP frame-ancestors directive. The way to set it up will depend on how they build their site. For nextjs it requires a middleware config: https://nextjs.org/docs/app/building-your-application/configuring/content-security-policy

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Many thanks for all these details, @remidej ! I've just updated the docs. Are we good to merge the PR this afternoon?

Copy link
Contributor

Choose a reason for hiding this comment

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

this is perfect 👍

@pwizla pwizla merged commit d302eda into main Dec 4, 2024
4 checks passed
@pwizla pwizla deleted the repo/static-preview branch December 4, 2024 16:24
@pwizla pwizla added temp: port to doc6 (Temporary label) The content of this PR should be ported to Strapi Docs v6 and removed temp: port to doc6 (Temporary label) The content of this PR should be ported to Strapi Docs v6 labels Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: new content PRs for new product features or new documentation sections source: Dev Docs PRs/issues targeting the Developer Docs source: User Guide PRs/issues targeting the User Guide
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants