WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 14 months ago

#37011 closed enhancement

Don’t link custom logo if it’s displayed on the front page — at Version 17

Reported by: FlorianBrinkmann Owned by: joedolson
Milestone: 5.5 Priority: highest omg bbq
Severity: blocker Version: 4.6
Component: Themes Keywords: has-patch has-dev-note commit dev-reviewed
Focuses: accessibility Cc:

Description (last modified by SergeyBiryukov)

Hey!

I have looked at the get_custom_logo() function recently and noticed that the logo is linked to home even if it is displayed on the front page.

Wouldn’t it make sense to only link the logo if it is not displayed on the front page?

Rian Rietveld writes:

On the homepage the logo or site title can be put into an H1. This is not a link, because that will be a link to the page itself.

And I think it makes perfect sense :)

Change History (22)

@Soean
5 years ago

First patch

#1 @Presskopp
5 years ago

  • Keywords has-patch added

#2 @sabernhardt
18 months ago

  • Focuses accessibility added
  • Keywords needs-refresh added

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


18 months ago

#4 @joedolson
18 months ago

  • Component changed from General to Themes
  • Owner set to joedolson
  • Status changed from new to accepted

#5 @joedolson
18 months ago

  • Milestone changed from Awaiting Review to 5.5

#6 @afercia
18 months ago

This ticket was discussed during today's accessibility bug-scrub: it was somehow missed because set to the "General" component. Updating the ticket properties for better visibility and setting milestone to 5.5.

@audrasjb
18 months ago

If on home/front page, don’t link the logo to home

#7 @audrasjb
18 months ago

  • Keywords needs-refresh removed

37011.1.diff refreshes old patch and check both is_home() and is_front_page().

#8 @SergeyBiryukov
18 months ago

Thanks for the patch! Just noting that checking both is_home() and is_front_page() seems redundant here. If a page is set as a front page and another one as a posts page, then the link should only be removed on the former, not on the latter.

is_front_page() should be enough.

@sabernhardt
18 months ago

maintaining a container element, revising conditional logic, updating function information

#9 @sabernhardt
18 months ago

  • Keywords needs-testing needs-dev-note added

Related: #31027 began work toward removing the home page logo link in the Twenty Fifteen theme.

For the latest patch:

  1. The image needs a "custom-logo-link" container element so it can match the theme's styles of the linked logo on other pages. This patch keeps the same styling in at least Twenty Seventeen and Twenty Nineteen; logos in Twenty Fifteen and Twenty Twenty appear to be the same with or without the container element. Some of the themes that target the `a` element should still need updating for this change.
  2. The conditional logic is revised to keep the link on the blog page when using a static front page.
  3. The function description and since note are updated, and I'm sure they could be improved further.

It would be good to test in multisite, as well as more themes.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


18 months ago

#11 @samful
16 months ago

Tested the Core themes (Twenty Nineteen, Twenty Seventeen & Twenty Twenty) with 37011.2.diff on standard WordPress and Multisite WordPress and no issues here.

Setting the home page as a static page or leaving it as "Your latest Posts" seems to all work, the is_front_page() function seems to be doing it's job well.

On the Twenty Nineteen theme, the logo still has an on hover effect around it (a black circle) which may confuse some users, but I think this isn't a big issue as the cursor does not change, and so clearly isn't a hyperlink.

I agree that the some of the 'a' element themes may need changing after this patch, however after testing the number 1 on that list, the "Mesmerize" theme seems to still work as expected.

The since notes seems clear enough to me @sabernhardt , I think you've done a good job here.

Last edited 16 months ago by samful (previous) (diff)

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


16 months ago

#13 follow-up: @afercia
16 months ago

Should the logo alt attribute be empty on the front page?

I'd tend to think that when it's not within the link the logo is purely "decorative" and doesn't need its purpose to be described.

If so, I guess the alt should be explicitly set to empty string e.g.:

$custom_logo_attr['alt'] = '';

and the comment block above should be updated.

#14 in reply to: ↑ 13 @knutsp
16 months ago

Replying to afercia:

Should the logo alt attribute be empty on the front page?

I'd tend to think that when it's not within the link the logo is purely "decorative" and doesn't need its purpose to be described.

A logo is about visual identity, and hat is in my interpretation almost the opposite of. As I read https://www.w3.org/WAI/tutorials/images/decorative/ a logo is not such. It's not "Identified and described by surrounding text". The logo is usually there instead of the site title, usually a part of the H1 tag itself.

The alt text must be the site title.

#15 @SergeyBiryukov
16 months ago

Simplified some logic and adjusted documentation a bit.

37011.3.diff includes setting the alt attribute to an empty string for the home page, per comment:13.

37011.3.alt.diff doesn't include that change, since apparently there's no consensus yet.

#16 @audrasjb
16 months ago

  • Keywords 2nd-opinion added; needs-testing removed

Thanks for the refresh @SergeyBiryukov.
I'm personally inclined to ship 37011.3.alt.diff for now.

#17 @SergeyBiryukov
16 months ago

  • Description modified (diff)
Note: See TracTickets for help on using tickets.