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

Wrap logo in h1 tag for search engines and screenreaders #552

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ export const Logo = ({ src, loading = 'lazy', title, alt, href = '#' }) => {
return src ? (
<div className="logo">
<a href={href} title={title}>
<span>
<img src={src} alt={alt || title} title={title} loading={loading} />
</span>
<h1>
<span>
<img src={src} alt={alt || title} title={title} loading={loading} />
</span>
</h1>
</a>
</div>
) : null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,10 @@ a.user-menu-top-link {
font-size: 1em;
}

h1 {
margin: 0;
}

a {
color: inherit;
display: block;
Expand Down
7 changes: 7 additions & 0 deletions frontend/src/static/js/utils/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ export function renderPage(idSelector, PageComponent) {
const appSidebar = document.getElementById('app-sidebar');
const appContent = idSelector ? document.getElementById(idSelector) : undefined;

while (appHeader.firstChild) {
appHeader.removeChild(appHeader.firstChild);
}
while (appSidebar.firstChild) {
appSidebar.removeChild(appSidebar.firstChild);
}

if (appContent && PageComponent) {
ReactDOM.render(
<AppProviders>
Expand Down
4 changes: 2 additions & 2 deletions templates/components/header.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div id="app-header"></div>
<div id="app-header"><h1>{{ PORTAL_NAME }}</h1></div>

{% comment %}
Uncomment, or replace with your own cookie consent code.
Expand All @@ -17,4 +17,4 @@
</script>


{% endcomment %}
{% endcomment %}
27 changes: 26 additions & 1 deletion templates/components/sidebar.html
Original file line number Diff line number Diff line change
@@ -1 +1,26 @@
<div id="app-sidebar"></div>
<div id="app-sidebar">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you adding these at that place? I think it's better to avoid bringing all of them here for a number of reason (they might be removed by some installations, or even the URLs changed etc)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly because that's where they are generated in the frontend, so when they're replaced during load, the markup is similar. This means it's compatible with the existing stylesheet to prevent it from blinking and rearranging as the page progressively loads on slower connections. The differences from the final frontend-generated version also mean that any search engines indexing the page which do partially evaluate JavaScript (such as Google) don't ding you for presenting something different to users than what the search engine sees.

<nav>
<ul>
<li><a href="/">Home</a></li>
<li><a href="/featured">Featured</a></li>
<li><a href="/recommended">Recommended</a></li>
<li><a href="/members">Members</a></li>
<li><a href="/latest">Recent uploads</a></li>
<li><a href="/tags">Tags</a></li>
<li><a href="/categories">Categories</a></li>
</ul>
</nav>
<nav>
<ul>
<li><a href="/history">History</a></li>
<li><a href="/liked">Liked media</a></li>
</ul>
</nav>
<nav>
<ul>
<li><a href="/about">About</a></li>
<li><a href="/tos">Terms</a></li>
<li><a href="/contact">Contact</a></li>
</ul>
</nav>
</div>