search

skeletonlabs / create-skeleton-app

CLI installer for Skeleton apps and monorepo for the sites that form the templates
16 stars 5 forks source link

Bare Bones comes with half the App Shell configuration #34

Closed Hugos68 closed 1 year ago

Hugos68 commented 1 year ago

If I'm not mistaken 2 things need to be configured in order for the AppShell to work correctly:

<body>
    <div style="display: contents" class="h-full overflow-hidden">%sveltekit.body%</div>
</body>

and

html, body { @apply h-full overflow-hidden; }

When selecting the Bare Bones template only 1 of the options get imported while the App Shell not even being used. So I'd like to request a removal of the app.postcss bit since it really messed me up when trying to implement sticky headers myself since the app.postcss was causing issues even though I never intended on using AppShell. If for some reason it's decided to say. please add the above bit to the app.html aswell then, otherwise a half configured AppShell config would be present.

endigo9740 commented 1 year ago

I'd suggest the h-full remain for both - it has little downsides and several benefits, but overflow-hidden be removed on barebones. It can be detrimental if the page needs to scroll. We only implement the overflow setting for App Shell because the shell itself handles scrolling and we want want to end up with double scrollbars.

Though if this truly is barebones, we should probably match the SvelteKit defaults.

endigo9740 commented 1 year ago

@niktek @Hugos68 After reading through the discussion on Discord I wanted to expand on my message above with a few suggestions:

Additionally I think there's some benefit to making the "Welcome" template be more semantic, call it "App Shell Starter" or something like that. Something to give semantics for what it adds over the barebones base.

We might also introduce third option which is custom layout or something like that, that provides some minimal configuration towards a custom layout. I'm happy to jump in an helps with these changes as I can.

Bare minimum those barebones changes need to be made. A couple of those changes are prepping for the app shell and optional theme settings - which is far removed from the semantic name.

SubjectiveTeam commented 1 year ago

@niktek @Hugos68 After reading through the discussion on Discord I wanted to expand on my message above with a few suggestions:

* If we're going to call the base template "barebones" we should basically provide the absolutely minimal configuration needed

* This means no `data-theme`

* No `app.html` modifications beyond what SvleteKit provides

* No `app.postcss` modifications

Additionally I think there's some benefit to making the "Welcome" template be more semantic, call it "App Shell Starter" or something like that. Something to give semantics for what it adds over the barebones base.

We might also introduce third option which is custom layout or something like that, that provides some minimal configuration towards a custom layout. I'm happy to jump in an helps with these changes as I can.

Bare minimum those barebones changes need to be made. A couple of those changes are prepping for the app shell and optional theme settings - which is far removed from the semantic name.

I think that's a great idea, there is no limit to templates so having a seperate bare bones to started would be a very nice to have thing.

Oops Responded on the wrong github account, this is Hugos but on a different acc

Mahmoud-zino commented 1 year ago

I would like to work on this issue. also, it would be very helpful if we define exactly what needs to be done since the comments above are just suggestions. 🙏

niktek commented 1 year ago

Fixed and closing