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

AppShells and Layouts #158

Closed
niktek opened this issue Aug 26, 2022 · 11 comments · Fixed by #174
Closed

AppShells and Layouts #158

niktek opened this issue Aug 26, 2022 · 11 comments · Fixed by #174
Assignees
Labels
documentation Improvements or additions to documentation feature request Request a feature or introduce and update to the project. ready to test Ready to be tested for quality assurance.
Milestone

Comments

@niktek
Copy link
Contributor

niktek commented Aug 26, 2022

Will be fleshing this out more - but with AppShells on the roadmap and also needing to revise my own site, I ran into the issue of svelte layouts not being able to have named slots as per sveltejs/kit#627 which also has implications for Skeleton offering a nice OOB DX.

@endigo9740
Copy link
Contributor

endigo9740 commented Aug 26, 2022

Hmm that's a bummer. Hopefully that's something they address. It would be hugely beneficial for this use case.

FYI, see the Roadmap > Docs > Doc Refresh section here:
https://github.com/Brain-Bones/skeleton/wiki/%F0%9F%9B%A3%EF%B8%8F-The-Skeleton-Roadmap-(proposal)

Part of that first bullet is "improved layout". I was kind of expecting to handle setup for the Nav Bar and App Shell during the update to the Skeleton docs site. I essentially want to try and "dogfood" every component we can within the Docs site before we release it. This way we know it works well in a production environment.

Given the issue you linked and the placement on the roadmap proposal, I'd say this one may be a ways out. But hopefully the named slot stuff is sorted by then! It may all work out.

@endigo9740
Copy link
Contributor

endigo9740 commented Aug 26, 2022

The alternative, however, would be to treat the App Shell like a component, rather than a layout for now. You have to import and set it up on each layout, but that may still be better than just rebuilding page layouts for scratch over and over. I'll think this about this!

@endigo9740 endigo9740 added documentation Improvements or additions to documentation feature request Request a feature or introduce and update to the project. labels Aug 26, 2022
@endigo9740
Copy link
Contributor

endigo9740 commented Aug 27, 2022

Linking this for reference later. I think something like this might be helpful for generating the AppShells layout structure:
https://css-tricks.com/using-grid-named-areas-to-visualize-and-reference-your-layout/

This was referenced Aug 29, 2022
@endigo9740
Copy link
Contributor

endigo9740 commented Aug 29, 2022

FYI this is still in progress but in order to use SvelteKit's routing group layouts I needed to update dependencies:
https://kit.svelte.dev/docs/advanced-routing#advanced-layouts-group
https://dev.to/parables/whats-new-in-svelte-kit-100-next445-group-layout-1ld5

Screen Shot 2022-08-29 at 1 48 06 PM

@endigo9740
Copy link
Contributor

endigo9740 commented Aug 29, 2022

Great progress made on this today!

Notable improvements:

  • We now have a dedicated AppBar (read: top nav bar) component
  • We now have an AppShell component that houses left/right panels, among other odds and ends
  • The homepage has a unique layout and design, similar to Vite, Vitest, Tailwind, etc.
  • The root layout now only sets the header/content area - it's MUCH cleaner than before
  • I've setup a (inner) Route Group with it's own layout, this implements the AppShell component
  • We now have a /libs/_documentation section for doc-specific components.
  • These doc-specific components include instances of AppBar, Drawer, Footer, and ThemeGeneration components
  • All inner pages (guides, etc) include the new shared footer as well

When I return to this I'll look to flush out the homepage a bit more. There's a couple links that need to be fixed, and I need to set it up to fetch the list of contributors and their avatars. GitHub provides an API for this, but I'll need to explore it a bit deeper - needs an auth token, etc.

(NOTE: images removed, see the next update below)

@endigo9740
Copy link
Contributor

endigo9740 commented Aug 30, 2022

Here's a video showing the current layout for the homepage versus inner pages:
https://i.imgur.com/dReerEo.mp4

General idea is the AppShell provides a ton of potential slots:

NOTE: the sidebars (in red) will most likely have a default width of 'auto'. This means they will collapse when not populated, but allow content to be rendered if need be, non-visibly

Screen Shot 2022-08-30 at 12 42 19 PM

Here's the root layout markup:

Screen Shot 2022-08-30 at 12 41 48 PM

Versus the (inner) route group layout:

Screen Shot 2022-08-30 at 12 49 14 PM

Note the current AppShell will handle the "universal" style layout. This should handle MOST traditional layouts. However, it may be possible to add more exotic "shells" (read: layouts) with variant components in the future.

@endigo9740 endigo9740 added this to the v1.0 milestone Aug 31, 2022
@endigo9740 endigo9740 self-assigned this Aug 31, 2022
@endigo9740
Copy link
Contributor

endigo9740 commented Aug 31, 2022

Documentation added for the App Shell component. The props are pretty minimal on this one, but we may find cases where it'll be beneficial to add more.

In case you're curious, here's how I'm hiding the left sidebar on the Homepage route:

$: sidebarLeftWidth = $page.url.pathname === '/' ? 'w-0' : 'lg:w-auto';
<AppShell {sidebarLeftWidth}>...</AppShell>

I'd welcome suggestions if you have any @niktek .

image

@endigo9740
Copy link
Contributor

Vitest integration cases have been added. I think with this the component is ready for QA!

Please pull branch feat/app-shell to begin testing asap @thomasbjespersen @niktek

@endigo9740 endigo9740 added the ready to test Ready to be tested for quality assurance. label Aug 31, 2022
@endigo9740 endigo9740 linked a pull request Sep 1, 2022 that will close this issue
@endigo9740
Copy link
Contributor

Re-opening this so QA can continue.

@endigo9740 endigo9740 reopened this Sep 1, 2022
@endigo9740
Copy link
Contributor

endigo9740 commented Sep 5, 2022

FYI I constructed a simple Sveltekit app and implemented the AppShell, AppBar, and Drawer components. Everything worked as expected following the dev branch install instructions paired with a local .tgz tarball version of the Skeleton package.

Screen Shot 2022-09-05 at 3 25 18 PM

However I did run into two small issues...

First, an ARIA Heading error log in the terminal when running npm run test:

/.../node_modules/@brainandbones/skeleton/GradientHeading/GradientHeading.svelte:11:99 A11y: Elements with the ARIA role "heading" must have the following attributes defined: "aria-level"

Second, I noted the Drawer "fly" animation is missing, which means the drawer only fades in rather than slide + fade. Compare the gifs of the Docs site animation version the test app using the package:

I'll create/update tickets accordingly for these.

@endigo9740
Copy link
Contributor

Released as part of v0.37.32!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation feature request Request a feature or introduce and update to the project. ready to test Ready to be tested for quality assurance.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants