skeletonlabs / skeleton

A complete design system and component solution, built on Tailwind.
https://skeleton.dev
MIT License
4.9k stars 305 forks source link

AppShell scrolling accessibility issues #1408

Closed benjamin-brady closed 1 year ago

benjamin-brady commented 1 year ago

Current Behavior

AppShell has poor scrolling accessibility due to the scroll bar being isolated to a child element rather than the window/page. Page up/down keys and mouse scrolling does not work until the container div is focused, and there is no visual indicator that it is focused. Anecdotally I've found in the past users complain about child element scrolling and I've had no complaints since I changed my sites to scroll on the window.

Possible solution:

The header and two sidebars will need to be position: fixed. The tricky part is how to maintain the original size/displacement and ideally without the developer having to pass dimensions to the AppShell component. Also, in the CLI welcome template html,body should not have a height and overflow set.

I used this global css to fix the issue but obviously a more elegant solution is needed.

<style global>
    body, html {
        height: initial;
        overflow:initial;
    }

    #appShell header {
        position: relative;
        /* This bit is tricky. How to calculate it automatically based on content size? */
        height: 3rem;
        overflow:initial;
    }

    .app-bar {
        position: fixed;
                width: 100%;
    }
</style>

Reproduction / Steps To Reproduce

  1. Visit https://www.skeleton.dev/blog - don't click anything
  2. Press page up/down keys - no scrolling
  3. Hover the mouse over the header - no scrolling

Anything else?

Popular sites like Google, YouTub, Reddit and even this Github page have sticky headers and window level scrolling so it's somewhat a de facto convention when implementing a sticky header.

Sarenor commented 1 year ago

Thanks for the accessibility note. You're right and I can't speak for the reasons for implementing it like this since I didn't implement it. Chris is very accessibility focused though and if he decided that the best way to implement the AppShell is this way I'd give him the benefit of the doubt.

As usual, the AppShell is a suggestion and an offering. You're free to use it and if it doesn't fit your usecase, you can always built your own layout.

endigo9740 commented 1 year ago

Unfortunately there's nothing here that wasn't already on my radar. I'm not happy with how it operates either. My hope was that it would be a tool that absolute beginners could use to get off the ground, but it's kind of turned into a crutch that everyone defaults to.

I'll be putting some thought into how we can handle this, but for now, know that I agree with you and would like to see an improvement here.

benjamin-brady commented 1 year ago

As usual, the AppShell is a suggestion and an offering. You're free to use it and if it doesn't fit your usecase, you can always built your own layout.

I think the concept of an app shell actually does fit my use case but the implementation could be much better. I could reinvent a lot of things but that would be tedious hence I'm using a framework to save me time and focus on the meat of my app.

My hope was that it would be a tool that absolute beginners

As someone new to Skeleton I'm using a framework to ensure my site is using best practices. I've implemented a sticky header before but I thought I'd do it the skeleton way because that would be best practice. The first thing I did after creating a new project was to search for how to create a responsive layout with a mobile friendly menu which landed me on this skeleton blog which uses AppShell.

endigo9740 commented 1 year ago

Yeah, I mean it has it's place. It's just a shame that SvelteKit layouts do not currently support named slots. This would help simplify things a bit. The component embedded in the layout, and then everything being juggled around that causes some of these issues. Fixed or scrollable headers/footer are easy enough as well. It's the fixed sidebars where a bit more of the challenge comes in.

All this said, we'll keep this ticket as a goalpost for revisiting the app shell and looking to implement improvements. This could improve a lot of thing, even the upcoming Table of Contents updates I have in mind. Just know this may not be a quick turn around given the current agenda though. So I will ask folks keep their expectations reasonable right now. It may take a while to get back to this.

richardvanbergen commented 1 year ago

My hope was that it would be a tool that absolute beginners could use to get off the ground, but it's kind of turned into a crutch that everyone defaults to.

@endigo9740 Maybe a good solution would be to simply express that in the docs somehow. I'm experienced in Web Dev but completely new to Skeleton UI and I kind of just assumed it was a cool little utility we can use to build a layout. It wasn't until I started having troubles with the scrolling that I found this thread.

While the solution (build your own layout) is completely reasonable, if you're new to the project then that's not very obvious.

❤️ Your guys work by the way.

endigo9740 commented 1 year ago

So I've spent a bit of time testing this today, and so far I'm not coming up with a better way to handle the App Shell that greatly improves upon how it's currently implemented.

Here's a version of the layout generated using vanilla CSS - though note I'm using Skeleton Element classes for the elements sinerted into child section on the page to visualize.

/src/app.html

<body>
    <div style="display: contents">%sveltekit.body%</div>
</body>

/src/app.postcss

html,
body {
    height: 100%;
}

/src/routes/+layout.svelte

<div id="app-shell">
    <header id="header">
        <div class="card variant-filled-primary p-4">(header)</div>
    </header>
    <div id="sidebarLeft">
        <div class="card variant-filled-secondary p-4 h-full">(sidebarLeft)</div>
    </div>
    <div id="sidebarRight">
        <div class="card variant-filled-tertiary p-4 h-full">(sidebarRight)</div>
    </div>
    <div id="page">
        <header id="pageHeader">
            <div class="card variant-filled-success p-4">(pageHeader)</div>
        </header>
        <main id="pageContent">
            <slot />
        </main>
        <footer id="pageFooter">
            <div class="card variant-filled-warning p-4">(pageFooter)</div>
        </footer>
    </div>
    <footer id="footer">
        <div class="card variant-filled-error p-4">(footer)</div>
    </footer>
</div>

<style lang="postcss">
    #app-shell {
        height: 100%;
        /* --- */
        display: grid;
        grid-template-columns: auto 1fr auto;
        grid-template-rows: auto 1fr auto;
        gap: 0;
        grid-auto-flow: row;
        grid-template-areas:
            'header header header'
            'sidebarLeft page sidebarRight'
            'footer footer footer';
    }

    #header {
        grid-area: header;
    }

    #sidebarLeft {
        grid-area: sidebarLeft;
    }

    #sidebarRight {
        grid-area: sidebarRight;
    }

    #page {
        display: grid;
        grid-template-columns: 1fr 1fr 1fr;
        grid-template-rows: auto 1fr auto;
        gap: 0px 0px;
        grid-auto-flow: row;
        grid-template-areas:
            'pageHeader pageHeader pageHeader'
            'pageContent pageContent pageContent'
            'pageFooter pageFooter pageFooter';
        grid-area: page;
        overflow-y: auto;
    }

    #pageHeader {
        grid-area: pageHeader;
    }

    #pageContent {
        grid-area: pageContent;
    }

    #pageFooter {
        grid-area: pageFooter;
    }

    #footer {
        grid-area: footer;
    }
</style>

And here's how it looks scrolled to the top and bottom of the page region:

Screenshot 2023-05-12 at 12 12 55 PM

Screenshot 2023-05-12 at 12 13 02 PM

To summarize, I'm struggling to find a way to create a complex layout like this that allows the main page content to remain in the window scope and allows for page up/down without clicking to focus that region of the page. Sticky headers and footers are simple enough, but it's the sidebars that make this challenging.

Now there are ways to implement fixed sidebars that don't interrupt the flow of the root window, but they have lots of downsides. Such as requiring a fixed absolute positioning, the sidebar must have a fixed width, and the<main> element must have a matching left margin applied to offset the space covered by the sidebar. This gets super convoluted when you start introducing dynamic headers and footers.

Here's an example showcasing the above: https://www.w3schools.com/howto/howto_css_fixed_sidebar.asp

All in all, I'm not sure this is a limitation of what we're doing, but rather a limitation of current layout capabilities on the web. That said, I don't claim to have all the answers here. I welcome input or suggestions from others if they have a better way to tackle this. However, we must meet the following criteria to have feature parity with the current App Shell:

  1. It needs to have dynamic regions (headers, footers, page regions, etc)
  2. All sections but the main page content region should collapse and be invisible when no content is present - ideally when no content is passed within that slot or the slot content has no width/height
  3. Should still be easily configurable with props, but with sane defaults out of the box

Again, suggestions welcome here.

endigo9740 commented 1 year ago

I've kept this task open a bit to field additional input on my findings in the post above. No comments have come through, so we'll go ahead and sunset this for now. I do encourage folks to test and consider providing suggestions for additional options here. We'll gladly reopen this if a new idea comes through.

The App Shell is not perfect, but it's but one option available to users. We're looking to take steps to make the barebones offering meet expectations soon. This will mean we're taking a softer stance towards recommending the App Shell.

https://github.com/skeletonlabs/create-skeleton-app/issues/34