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

Test and update docs related to app.html wrapping div #556

Closed
3 tasks
endigo9740 opened this issue Nov 15, 2022 · 2 comments · Fixed by #561
Closed
3 tasks

Test and update docs related to app.html wrapping div #556

endigo9740 opened this issue Nov 15, 2022 · 2 comments · Fixed by #561
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@endigo9740
Copy link
Contributor

Link to the Page

https://www.skeleton.dev/guides/frameworks/sveltekit

Describe the Issue

From user @cokakoala on Discord:

howdy folks, I just saw this new release of create-svelte (https://github.com/sveltejs/kit/releases/tag/create-svelte%402.0.0-next.191) pushed a couple of hours ago that now inlines style="display: contents" on the div that wraps %sveltekit.body% by default. I'm not sure if that would break anything when using App Shell since I haven't tried it out yet but I just wanted to give you guys a heads up

Per MDN:
https://developer.mozilla.org/en-US/docs/Web/CSS/display

These elements don't produce a specific box by themselves. They are replaced by their pseudo-box and their child boxes. Please note that the CSS Display Level 3 spec defines how the contents value should affect "unusual elements" — elements that aren't rendered purely by CSS box concepts such as replaced elements. See Appendix B: Effects of display: contents on Unusual Elements for more details.
Due to a bug in browsers, this will currently remove the element from the accessibility tree — screen readers will not look at what's inside. See the Accessibility concerns section below for more details.

MDN a11y concerns:
https://developer.mozilla.org/en-US/docs/Web/CSS/display#accessibility_concerns

Current implementations in most browsers will remove from the accessibility tree any element with a display value of contents (but descendants will remain). This will cause the element itself to no longer be announced by screen reading technology. This is incorrect behavior according to the CSS specification.

Essentially we need to test the following:

  • Will this suffice instead of needing users to modify the app.html wrapping div styles for App Shell and general layouts?
  • If so, update the SvelteKit/Vite frameworks guides
  • Additionally, update the App Shell documentation.

Are you able to create a Pull Request with the fix?

Yes

@endigo9740 endigo9740 added documentation Improvements or additions to documentation enhancement New feature or request labels Nov 15, 2022
@endigo9740
Copy link
Contributor Author

@niktek Here's the results:

  1. Using only the class styles we specified in the current docs - this works as expected!

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

  1. Using ONLY the new default style tag - unfortunately content does not fill the width. So we will need doc updates.

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

  1. Keeping the default display style but adding our recommended classes - this works as expected:

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

So #3 will be our answer. I'll update the docs accordingly. Feel free to update the CLI to match.

@endigo9740
Copy link
Contributor Author

I've updated both the Frameworks > SvelteKit guide and the App Shell docs with this new recommendation. I didn't fix our app.html file, but it doesn't really matter. This new style doesn't really do much as far as I can tell.

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 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant