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

Preview tokens 5: Version-based rendering #6825

Merged
merged 1 commit into from
Dec 2, 2024

Conversation

lukasbestle
Copy link
Member

@lukasbestle lukasbestle commented Dec 1, 2024

🚨 Merge first

Description

Summary of changes

$page->render() is now responsible to choose the correct version to render.

Users can at any time pass a custom version ID that will always be used if passed. Otherwise the version ID is auto-detected.

If a render version is already set globally, this is the first detection priority. This can be relevant if a template calls $somePage->render() of another page. By using the global render version ID, that rendered version will match the one of the "outer" page.

Otherwise the version ID will be taken from the request. In this PR this only re-implements the draft tokens.

Reasoning

  • Usage flexibility
  • DRY code

Additional context

  • Logic is moved over 1:1 for now. Following PRs 7-9 will implement the actual version tokens.
  • Tests need to disable caching in this PR because the current caching logic would make those tests fail otherwise. PR 6 will fix this and delete the temporary TODO additions to the tests.

Changelog

Enhancements

  • It is now possible to override the version ID in $page->render() with the new $versionId parameter.

Refactoring (since alphas)

  • Move request-based version detection from the App class closer to the action in $page->render()

Docs

We could document the new $versionId argument to $page->render(). But I don't think we already have a place in the guide or cookbook that uses $page->render(), at least I couldn't find one.

Ready?

  • In-code documentation (wherever needed)
  • Unit tests for fixed bug/feature
  • Tests and CI checks all pass

For review team

  • Add lab and/or sandbox examples (wherever helpful)
  • Add changes & docs to release notes draft in Notion

@lukasbestle lukasbestle added type: enhancement ✨ Suggests an enhancement; improves Kirby type: refactoring ♻️ Is about bad code; cleans up code labels Dec 1, 2024
@lukasbestle lukasbestle added this to the 5.0.0-beta.1 milestone Dec 1, 2024
@lukasbestle lukasbestle self-assigned this Dec 1, 2024
Base automatically changed from v5/changes/preview-tokens-4 to v5/develop December 2, 2024 08:18
Copy link
Member

@bastianallgeier bastianallgeier left a comment

Choose a reason for hiding this comment

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

It all looks very clean to me. But a second pair of eyes from @distantnative would be good :)

@bastianallgeier bastianallgeier merged commit bad6733 into v5/develop Dec 2, 2024
11 checks passed
@bastianallgeier bastianallgeier deleted the v5/changes/preview-tokens-5 branch December 2, 2024 13:20
@bastianallgeier
Copy link
Member

We mention the render method in the routing guide: https://getkirby.com/docs/guide/routing I'm not really sure if it actually fits there to document the $versionId argument.

But we should definitely add an example here: https://getkirby.com/docs/reference/objects/cms/page/render

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement ✨ Suggests an enhancement; improves Kirby type: refactoring ♻️ Is about bad code; cleans up code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants