Skip to content

Conversation

@maxfenton
Copy link
Contributor

@maxfenton maxfenton commented Dec 1, 2025

@maxfenton maxfenton requested a review from joshuapease December 1, 2025 16:24
@maxfenton
Copy link
Contributor Author

@joshuapease This is a medley of updates. Feel free to pick and choose or reject this PR

Copy link
Contributor

@joshuapease joshuapease left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Wanna cherry-pick this commit onto your branch too?

a67da11#diff-9b414e2b2d19da12e2a4f2dad41c293fad185c1366d4825ada5e8e27a9aac3dc

It has all of the template changes needed for the parts kit.

Forgot I had made this branch

])
;
'@web' => App::env('PRIMARY_SITE_URL'),
'@primarySiteUrl' => StringHelper::removeRight(App::env('PRIMARY_SITE_URL'), '/'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a change to the site's project config that needs to happen here too?

maxfenton and others added 2 commits December 4, 2025 11:52
@maxfenton
Copy link
Contributor Author

@joshuapease

  • cherry picked that commit with Parts Kit templates
  • updated the Site config to use the @primarySiteUrl instead of the $PRIMARY_SITE_URL env variable which lets us make specific adjustments in config/general.php to force or remove a trailing slash. This came up on sites where we're appending /es for a Spanish site and on Servd where the trailing slash was necessary:
'@primarySiteUrl' => StringHelper::ensureRight(App::env('PRIMARY_SITE_URL'), '/'),
'@spanishSiteUrl' => StringHelper::ensureRight(App::env('PRIMARY_SITE_URL'), '/') . 'es',

@maxfenton maxfenton requested a review from joshuapease December 4, 2025 16:55
Does not need {% layout %} and {% block %} tags.
Copy link
Contributor

@joshuapease joshuapease left a comment

Choose a reason for hiding this comment

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

I ran this PR though Cursor which is something I do lately.

It pointed out that I'd failed to migrated some of the parts kit templates.

Simplest thing was to push a commit to this PR with the remaining changes.

47234f9

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah... I realized we don't even have this plugin installed by default on the project.

With some of it's performance issues we've encountered (PHP has to download & parse JSON over S3, similar to the SVG over S3 issue), I'd recommend avoiding this as a default.

I do think it would be really cool to have repo or Notion doc with common configs & coding recipes. Not sure the place to keep that info

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