search

payloadcms / payload-3.0-demo

The official demo for Payload 3.0
https://next-payload-3-0-test.vercel.app
433 stars 140 forks source link

custom route is always wrapped by `MinimalTemplate` #216

Closed PoiScript closed 4 months ago

PoiScript commented 4 months ago

Hello, I'm trying to merge my project to payload 3.0, and I'm really impressed since everything just works out of the box.

However I finds out that all my custom route are wrapped by a MinimalTemplate. After digging into the source code, it seems that templateType variable in getViewFromConfig function is always set to minimal for custom view:

image

Is there a way to configure the custom routes to use the default template instead of minimal?

jacobsfletch commented 4 months ago

Hey @PoiScript this should be fixed with https://github.com/payloadcms/payload/pull/6379. Custom views are not supposed to be wrapped with a template at all. The correct pattern here is to render your own template (i.e. import the DefaultTemplate from @payloadcms/ui) into your custom views as needed. It's possible we could expose template as a config option as you've suggested, but maybe we can revisit this in the future.

r1tsuu commented 4 months ago

Hey @PoiScript this should be fixed with payloadcms/payload#6379. Custom views are not supposed to be wrapped with a template at all. The correct pattern here is to render your own template (i.e. import the DefaultTemplate from @payloadcms/ui) into your custom views as needed. It's possible we could expose template as a config option as you've suggested, but maybe we can revisit this in the future.

Hi, have you guys seen this https://github.com/payloadcms/payload/pull/5846 ? it adds templateType option to the views config.

jacobsfletch commented 4 months ago

Hi, have you guys seen this https://github.com/payloadcms/payload/pull/5846 ?

I have not, but good work. If you can resolve those merge conflicts, and simplify as much as possible, we might be able to merge it in. Let's at least get this fix in place for the time being. Can you link this issue from your PR?

One thing to ensure with this is that the user can still select 'none' as an option, and render their own just as they always could before.

r1tsuu commented 4 months ago

Hi, have you guys seen this payloadcms/payload#5846 ?

I have not, but good work. If you can resolve those merge conflicts, and simplify as much as possible, we might be able to merge it in. Let's at least get this fix in place for the time being. Can you link this issue from your PR?

One thing to ensure with this is that the user can still select 'none' as an option, and render their own just as they always could before.

yeah, 'none' option is allowed, here are e2e's https://github.com/payloadcms/payload/pull/5846/files#diff-3d21c5796617c0eff926345ff22f90f5c226642985fb3e5e8808e4758836c4fd will resolve merge conflicts

jacobsfletch commented 4 months ago

A fix for this will go out in the next release. Feel free to bump your PR and mark it with the new feature label. Opening a new discussion around your feature also helps.