GoogleChrome / lighthouse

Automated auditing, performance metrics, and best practices for the web.
https://developer.chrome.com/docs/lighthouse/overview/
Apache License 2.0
28.24k stars 9.35k forks source link

[SEO Audits] Tap targets are sized appropriately #4358

Closed rviscomi closed 5 years ago

rviscomi commented 6 years ago

Audit group: Mobile friendly Description: Tap targets are sized appropriately Failure description: Tap targets are not sized appropriately Help text: Interactive elements like buttons and links should be large enough, and have enough space around them, to be easy enough to tap without overlapping onto other elements. Learn more.

Success conditions:

If possible, the weighting for this audit should be proportional to the number of interactive element failures.

Interactive elements:

For reference, see: https://developers.google.com/web/fundamentals/accessibility/accessible-styles#multi-device_responsive_design

rviscomi commented 6 years ago

When this issue is resolved let's also remove the manual audit for mobile friendly test.

kdzwinel commented 6 years ago

I finished the audit, but after testing had to add couple of tweaks:

@rviscomi let me know if these SGTY


As mentioned, I started testing on real websites (some of the Alexa's top500). Unfortunately, the results are similar to the font-size v1. Everyone fails and they fail bad 😢

See: https://imgur.com/a/0YoNY (tap targets are marked with: red - too small, yellow - good size but too close, green - OK)

Some thoughts:

@rviscomi We may want to lax this audit a bit not to fail everyone. I don't know what was PSI team's algorithm for this, but they sure were not reporting that many failures. BTW PSI doesn't seem to report these issues anymore, nor does mobile-friendly test 🤔

rviscomi commented 6 years ago

Chatted offline about this. Some notes for posterity:

kdzwinel commented 6 years ago

Update

I adjusted the algorithm as discussed and now it boils down to:

place a "finger" (48px x 48px square) in the middle of the tap target and see if it intersects with any other tap targets

⚠️Note that we are no longer punishing for small tap targets.

Caveats identified

Results

Blue square represents a finger. Red rectangles are failing tap targets, green rectangles are passing tap targets.

⚠️ Footers, which usually have many colliding links, are dragging all results down.

Wikipedia main page - 9% of tap targets are too close

Wikipedia article - 43% of tap targets are too close

Apple main page - 17% of tap targets are too close

Google main page - 7% of tap targets are too close

Yahoo main page - 50% of tap targets are too close

WAPO article page - 28% of tap targets are too close

NYT article page - 36% of tap targets are too close

huffpost article page - 18% of tap targets are too close

addyosmani commented 6 years ago

Hey folks. We're currently evaluating what UX audits Lighthouse should be prioritizing and I recall tap-targets being pretty high in prio (and overlapped nicely with PSI parity). I'm aware @kdzwinel and @rviscomi had put a lot of time into researching efficacy of the implementation here - is there a status update on where we're at with this work?

rviscomi commented 6 years ago

Hey @addyosmani, I think we're very close and just need to resolve a few of the questions/caveats noted in the previous comments. We're still prioritizing v1 of the structured data audit, which we expect to ramp down by the end of the month, so we should pick this audit back up by then.

addyosmani commented 6 years ago

SG. Thanks for the update, @rviscomi!

rviscomi commented 6 years ago

Just to update, the SD audit is still being worked on so this hasn't been finished yet. @kdzwinel what is left to do for this audit and what's your ETA?

mattzeunert commented 5 years ago

For reference, here's how we currently display the tap target results:

screen shot 2018-10-06 at 16 52 09

I'll play around with making the selector more specific or maybe including the link/button CTA text.

Would probably also be good to call Math.floor() on the node dimensions.

rviscomi commented 5 years ago

+1 to including the inner text if possible. Another idea is to link the node in the Elements panel. Some targets may be <a><img></a> so we'll need a non-text option for that, perhaps the inner HTML.

The repeated "Target is too small" text is also redundant so I think we can just include the target's dimensions.

mattzeunert commented 5 years ago

This is what I have at the moment. The two types of tap target failures (too small/too close) are both in the same table right now, so we need some way to distinguish them.

screen shot 2018-10-06 at 18 10 30

rviscomi commented 5 years ago

Is it possible to have two tables in the same audit, one for each failure reason? Otherwise maybe we can add more columns, for example:

Element Failure Expected Actual
a.button ("Subscribe") Target is too small 48x48+ 105x36
a.devsite-product-name-link | a.devsite-breadcrumb-link ("Web") Targets are too close to each other 32px+ 0px
mattzeunert commented 5 years ago

We don't check for minimum size anymore, just tap targets being too close. I wasn't part of the original discussion, but I imagine that already detects a lot of tap targets that are too small, and if there are no other tap targets nearby maybe that's not a problem because the phone knows which one the user meant to tap on?

screen shot 2018-10-22 at 19 15 04
rviscomi commented 5 years ago

Ok, that makes the messaging easier if we're just focusing on one thing.

When showing the elements, are we able to make them linkable to the Elements panel in CDT when available? Anything to provide more context to the elements so developers can more easily identify and fix the issues.

Do we have any checks in place to limit the character length of the elements' HTML? I can imagine some elements having lots of children and making the table illegible.

When communicating how close the elements are, we should also be careful to say that the page was tested with specific viewport dimensions. The page may be laid out differently for different viewport sizes and the elements may change.

mattzeunert commented 5 years ago

Yeah we have code to link the outerHTML to the DOM node, but that feature is broken at the moment.

I don't think we do any truncation for the accessibility audit. Cutting off the text risks hiding important info like the text content, but limiting it to a high value like 1000 characters can't hurt. We could also try to show more digestible information than the element outerHTML – I think we use that because it's what axe-core provides.

mattzeunert commented 5 years ago

To illustrate the outstanding edge cases:

screen shot 2018-10-25 at 18 35 17

Title is broken over two lines, giving two finger areas which cause an overlap, even though clicking on the link is fine. (And in this case it would be a failure anyway because the content below is very close.)

screen shot 2018-10-25 at 18 37 11

Content overflows. Client rects/bounding client rects only cover original area. We can get the correct rect by selecting the contents and getting client rects on the selection. But in some cases selections can cover unexpected areas of the page. Might be fixable if we spend more time on it, but don't think it's essential initially.

screen shot 2018-10-25 at 18 40 13

Two things here: 1) "| Privacy" two client rects has been fixed, now has a single centered finger area 2) Google Developers image is outside the link area, so it's treated as a separate tappable area

rviscomi commented 5 years ago

Thanks for documenting these @mattzeunert. These would be good to include in any Lighthouse docs (cc @kaycebasques) but not necessarily launch-blocking v1 of the audit.

As discussed in the meeting today, let's move this forward adding tests and accounting for paragraph targets per WCAG.

mattzeunert commented 5 years ago

Status

New edge cases

Multiple finger areas are causing a lot of problems. Proposed solution:

1) Merge touching client rects together, similar to getBoundingClientRect. Will try using getBCR directly and see if it causes problems. We originally switched from getBCR to getClientRects to avoid an overly large target area for display: inline targets tags that span multiple lines. But that mostly happened in text blocks anyway. 2) Only put finger area on the largest client rect (e.g. in the last example in the image below, use the tap target of the image instead of checking each one)

screen shot 2018-11-01 at 10 43 48


Another issue is how we handle overflow: scroll elements. The elements exist in a certain position on the page, but don't actually overlap because they are contained with the parent element's scroll area.

This CNN article has a nav at the top that's outside the viewport, but the nav contains lots of scrollable content reaching down into the viewport. To fix that I marked tap targets that are within an overflow: scroll container but not with the container's BCR as invisible.

Now France24 puts almost all page content in a scrollable container, so most tap targets on that page are marked as invisible.

I think that's fine for now? Better to avoid penalizing than penalizing too much.

mattzeunert commented 5 years ago

Tried it on a couple sites and the scoring looks fine to me.

The audit will report as passed if the tap targets score is greater than 90 (see second image).

screenshot 2018-11-19 at 09 04 03

screenshot 2018-11-19 at 09 03 38