-
Notifications
You must be signed in to change notification settings - Fork 9
fix: move footer to main layout and improve contrast for WCAG compliance #542
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
Changes from 3 commits
934354b
b740121
34eaebf
94fe733
e2bbcf0
fe7c4b5
fc22616
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| <svg viewBox="0 0 16 16" aria-hidden="true"> | ||
| <use xlink:href="#svg-link"></use> | ||
| </svg> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| {%- comment -%} Include as: {%- include components/sidebar.html -%} Depends on: | ||
| page(?), site. Results in: HTML for the side bar. Includes: title.html, | ||
| components/site_nav.html, nav_footer_custom.html Overwrites: nav_footer_custom. | ||
| Should not be cached, because nav_footer_custom.html might depend on page. {%- | ||
| endcomment -%} | ||
|
|
||
| <div class="side-bar"> | ||
| <div class="site-header" role="banner"> | ||
| <a href="{{ '/' | relative_url }}" class="site-title lh-tight" | ||
| >{% include title.html %}</a | ||
| > | ||
| <button | ||
| id="menu-button" | ||
| class="site-button btn-reset" | ||
| aria-label="Toggle menu" | ||
| aria-pressed="false" | ||
| > | ||
| <svg viewBox="0 0 24 24" class="icon" aria-hidden="true"> | ||
| <use xlink:href="#svg-menu"></use> | ||
| </svg> | ||
| </button> | ||
| </div> | ||
|
|
||
| {% include_cached components/site_nav.html %} | ||
| </div> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| <!doctype html> | ||
| <html lang="{{ site.lang | default: 'en-US' }}"> | ||
| {% include head.html %} | ||
| <body> | ||
| <a class="skip-to-main" href="#main-content">Skip to main content</a> | ||
| {% include icons/icons.html %} {% if page.nav_enabled == true %} {% include | ||
| sidebar.html %} {% elsif layout.nav_enabled == true and page.nav_enabled == | ||
| nil %} {% include sidebar.html %} {% elsif site.nav_enabled != false and | ||
| layout.nav_enabled == nil and page.nav_enabled == nil %} {% include | ||
| sidebar.html %} {% endif %} | ||
|
|
||
| <div class="main" id="top"> | ||
| {% include components/header.html %} | ||
|
|
||
| <div class="main-content-wrap"> | ||
| {% include components/breadcrumbs.html %} | ||
| <div id="main-content" class="main-content"> | ||
| <main> | ||
| {% if site.heading_anchors != false %} {% capture raw_anchor_svg %} | ||
| {% include components/anchor_icon.html %} {% endcapture %} {% assign | ||
| anchor_svg = raw_anchor_svg | strip_newlines %} {% include | ||
| vendor/anchor_headings.html html=content beforeHeading="true" | ||
| anchorBody=anchor_svg anchorClass="anchor-heading" | ||
| anchorAttrs="aria-labelledby='%html_id%'" %} {% else %} {{ content | ||
| }} {% endif %} {% if page.has_toc != false %} {% include | ||
| components/children_nav.html %} {% endif %} | ||
| </main> | ||
| </div> | ||
| </div> | ||
|
|
||
| <footer class="site-footer"> | ||
| This site uses | ||
| <a href="https://github.com/just-the-docs/just-the-docs" | ||
| >Just the Docs</a | ||
| >, a documentation theme for Jekyll. | ||
| </footer> | ||
|
|
||
| {% if site.search_enabled != false %} {% include | ||
| components/search_footer.html %} {% endif %} | ||
| </div> | ||
|
|
||
| {% if site.mermaid %} {% include components/mermaid.html %} {% endif %} | ||
| </body> | ||
| </html> |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,3 +1,9 @@ | ||||||
| html, | ||||||
| body { | ||||||
| height: 100%; | ||||||
| margin: 0; | ||||||
| } | ||||||
|
|
||||||
| div.site-header { | ||||||
| max-height: 15em; | ||||||
| height: 15em; | ||||||
|
|
@@ -6,3 +12,19 @@ div.site-header { | |||||
| div.site-logo { | ||||||
| background-position: center center; | ||||||
| } | ||||||
|
|
||||||
| .main { | ||||||
| display: flex; | ||||||
| flex-direction: column; | ||||||
| min-height: 100%; | ||||||
| } | ||||||
|
|
||||||
| .main-content-wrap { | ||||||
| flex: 1; | ||||||
| } | ||||||
|
|
||||||
| .site-footer { | ||||||
| position: static; | ||||||
| width: 100%; | ||||||
| color: #6a686d; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixes colour contrast for light mode (🎉) but makes it worse in dark mode. (new: #6a686d contrast ratio 2:27:1) (old: #959396 contrast ratio 4:9:1) Pretty sure we need to be theme-agnostic here and I think using
Suggested change
I'm not sure about the styling though... that's probably a matter of taste. Do we prefer the footer to be a little bit less contrasty than the main body text? Another option is to investigate intercepting the theme colours which may anyway be needed for #535.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for noticing this! My bad, forgot to check it in dark mode. I think the body text color fits pretty well, though. |
||||||
| } | ||||||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! I'm sorry for doubting you, dear link-checker. I take back the yellow card.