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

Extract footer overhaul from #2215 #4003

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

syeopite
Copy link
Member

@syeopite syeopite commented Jul 24, 2023

With the recentish PRs of adding contact information and abuse links to Invidious's footer I thought I'd throw my hat back in the ring with my old footer overhaul code from #2215

footer

This is a very quick and dirty extraction with the only notable change being the updated locale keys to match how it's structured nowadays. Nothing new has been added for now. If this PR is to be accepted, the footer will probably also need a slight palette change to match the lighter tons of Invidious' UI compared to what I had back in #2215.

Comment on lines +116 to +118
<% if buffer_footer %>
<div id="footer_buffer"></div>
<% end %>
Copy link
Member

Choose a reason for hiding this comment

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

What is this footer buffer for?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a bit hard to explain but its mostly to give the contents some breathing room on select pages. It looks too cramped and shows too much information right-away imo. A block footer like so is intended to be scrolled down as to not overshadow the main contents of the page. See the examples below:

Without buffer (visible without scrolling):
without buffer

With buffer (requires scrolling):
with buffer

You can find some examples on sites like stackoverflow.com, vox.com, etc.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see! Though 50vh seems a bit too large. What about 1/4th of that?
Also, can you put the footer div only on the selected .ecr views, rather than in the template?

Comment on lines +179 to +194
<% if CONFIG.login_enabled %>
<li class="pure-menu-item footer-section-item">
<% if env.get? "user" %>
<form action="/signout?referer=<%= env.get?("current_page") %>" method="post">
<input type="hidden" name="csrf_token" value="<%= URI.encode_www_form(env.get?("csrf_token").try &.as(String) || "") %>">
<a href="#" title="<%= translate(locale, "Log out") %>">
<input style="all:unset" type="submit" value="<%= translate(locale, "Log out") %>">
</a>
</form>
<% else %>
<a href="/login?referer=<%= env.get?("current_page") %>" title="<%= translate(locale, "Log in") %>">
<%= translate(locale, "Log in") %>
</a>
<% end %>
</li>
<% end %>
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that log in/out needs to be in the footer, as it's already at the top-right on all pages. Sounds redundant to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mostly for designs reasons. Without the section with the "preferences" and "login" links I personally feel like too much weight is put on the Invidious column making it look unbalanced.

Copy link
Member

Choose a reason for hiding this comment

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

You could move the "Matrix" link in the "support" column, and add a configurable "ToS" link in the "legal" column?
Also, the version is currently missing. It could be placed at the bottom right of the footer, so that space usage is more even.

locales/en-US.json Outdated Show resolved Hide resolved
"footer_navigation_section_header": "Navigation",
"footer_home_link": "Home",
"footer_project_information_section_header": "Invidious",
"footer_project_homepage_link": "Project Homepage",
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about using "Website"? I'm worried that "Homepage" could be confused with the actual homepage of the instance.

Comment on lines +220 to +224
<li class="pure-menu-item footer-section-item">
<a href="https://invidious.io/donate" title="<%= translate(locale, "footer_donate_link")%>">
<%= translate(locale, "footer_donate_link") %>
</a>
</li>
Copy link
Member

Choose a reason for hiding this comment

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

Could the URL be customized by the admin? I.e, by default link to our own donate page, but if the admin adds their own URL, the "donate" link redirects to that URL instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm personally not opposed to it but one of the requirements for #2032 is an link for the project's donation page. Perhaps we could add another footer section? Something like:

looks off

Though imo something looks weird about that composition but I can't quite put my finger on why.

Copy link
Member

Choose a reason for hiding this comment

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

That needs to be discussed with @unixfox and @TheFrenchGhosty, but imo, we (as a project/team) don't need more donations than what we already get. Instance maintainers are the one paying expensive hardware and in need of financial support!

Comment on lines +507 to +515

##
## Email to contact the instance maintainer; used in a mailto: link within the footer.
##
## Accepted values: Email
## Default: <none>
##
# instance_maintainer_email:

Copy link
Member

Choose a reason for hiding this comment

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

If we add more customization options (contact e-mail, donate page, etc...) it might be a good thing to group all of that (plus the banner) under a dedicated config section, like customization or similar.

Copy link
Member Author

Choose a reason for hiding this comment

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

Huh good call! Invidious as of d956b18 already has two customization options if you also count modified_source_code_url. And with this PR, #4007, #3826, and possibly more in the future all adding additional fields, a dedicated customization group will certainly be a good idea.

Co-authored-by: Samantaz Fox <coding@samantaz.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants