report(redesign): tooltip for category description #8840

connorjclark · 2019-05-03T19:37:45Z

connorjclark · 2019-05-03T19:38:10Z

lighthouse-core/report/html/templates.html

-    <div class="lh-category-header__description"></div>
+    <div class="lh-score__gauge">
+      <!-- Will be added to .lh-gauge__label via renderCategoryHeader -->
+      <!-- <span class="tooltip-boundary">


should i make this a template

yup, i think so. include in this one and .remove() if there's no description?

paulirish · 2019-05-03T19:42:21Z

lighthouse-core/report/html/templates.html

-    <div class="lh-category-header__description"></div>
+    <div class="lh-score__gauge">
+      <!-- Will be added to .lh-gauge__label via renderCategoryHeader -->
+      <!-- <span class="tooltip-boundary">


yup, i think so. include in this one and .remove() if there's no description?

paulirish · 2019-05-03T19:43:04Z

lighthouse-core/report/html/report-styles.css

  --lh-group-icon-background-size: var(--lh-score-icon-background-size);
+  --lh-info-icon-background-size: 20px;
+  --lh-score-icon-background-size: 24px;


just -lh-score-icon-size ?

btw ur commenting on something i just reordered

i added --lh-info-icon-background-size

could rename stuff after all this tho

paulirish · 2019-05-03T23:42:29Z

lighthouse-core/report/html/report-styles.css

@@ -1289,7 +1291,7 @@
  position: absolute;
  display: none; /* Don't retain these layers when not needed */
  opacity: 0;
-  background: #ffffff;
+  background: var(--body-background-color);


paulirish · 2019-05-06T16:38:27Z

lighthouse-core/test/report/html/renderer/category-renderer-test.js is cranky.

paulirish

lgtm once tests are happy.

connorjclark · 2019-05-06T17:37:01Z

done. had to introduce a wrapper el so that the label el makes more sense :)

connorjclark · 2019-05-06T17:37:25Z

lighthouse-core/test/report/html/renderer/category-renderer-test.js

@@ -185,7 +185,7 @@ describe('CategoryRenderer', () => {
    const categoryDOM = renderer.render(category, sampleResults.categoryGroups);
    assert.ok(categoryDOM.querySelector('.lh-gauge__wrapper--plugin'));
    const label = categoryDOM.querySelector('.lh-gauge__label').textContent;
-    assert.equal(category.title, label);
+    assert.equal(label, category.title);


was backwards

was backwards

just for code correctness purposes, not a functional change in the test, I hope?

paulirish · 2019-05-06T19:00:04Z

can you resolve the conflict?

connorjclark · 2019-05-06T23:12:36Z

Not functional. Error message was wrong when the test was failing.

…

On Mon, May 6, 2019, 4:10 PM Brendan Kenny ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In lighthouse-core/test/report/html/renderer/category-renderer-test.js <#8840 (comment)> : > @@ -185,7 +185,7 @@ describe('CategoryRenderer', () => { const categoryDOM = renderer.render(category, sampleResults.categoryGroups); assert.ok(categoryDOM.querySelector('.lh-gauge__wrapper--plugin')); const label = categoryDOM.querySelector('.lh-gauge__label').textContent; - assert.equal(category.title, label); + assert.equal(label, category.title); was backwards just for code correctness purposes, not a functional change in the test, I hope? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#8840 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA7CAMVWFJGSEMJUS34OI63PUC3HNANCNFSM4HKWPGSA> .

connorjclark · 2019-05-23T16:54:17Z

merged master

next steps on this?

connorjclark · 2019-05-25T00:27:40Z

uh....bump it up? if we truly want to keep it under 70, we can open an issue to research how to do that

brendankenny · 2019-05-29T00:40:42Z

bump it up? if we truly want to keep it under 70, we can open an issue to research how to do that

I think that's fine. We've been hovering above 69KB for a while now, and adding things to the report renderer was going to do it sooner or later. 75KB?
The tooltips don't appear to be working right now, at least in the deployed example.
so, there are two states here: gauge+label for the scores header, gauge+label+optional-description for categories? I'm not entirely following what's needed with <a> vs not <a>, though.

Rather than passing the option into renderScoreGauge, can it go back to how it was before, adding a tooltip if there's a description in renderCategoryHeader() (ensuring the score header gauges don't get one).

If there's more needed to sort out the anchor business, maybe it needs to be split more. The different category vs scores-header gauges aren't really the same thing except the score gauge...we could have renderScoreGauge return only what's really shared between the two uses and move more functionality back to the call sites.

connorjclark · 2019-05-29T20:50:57Z

incorporated @brendankenny's feedback

patrickhulce · 2019-05-29T21:19:19Z

Sorry @hoten seems like this regressed again at some point in the feedback cycle.

connorjclark · 2019-05-29T21:52:40Z

oops, fixed that

patrickhulce

LGTM

patrickhulce · 2019-05-30T15:16:07Z

lighthouse-core/report/html/report-styles.css

@@ -1246,7 +1281,6 @@
 }

 .lh-category-header .lh-score__gauge {
-  max-width: 400px;


just unnecessary or do we want the gauges to get bigger than this?

unnecessary

patrickhulce · 2019-05-30T15:17:30Z

lighthouse-core/report/html/report-styles.css

@@ -1470,7 +1504,6 @@
  display: block;
  animation: fadeInTooltip 250ms;
  animation-fill-mode: forwards;
-  animation-delay: 850ms;


this removes this everywhere, we want that right? I would guess so because a lot of folks don't seem to know that they can hover over the error to get the tooltip 👍

yeah, I figured it doesn't make sense to inject 850ms of latency for this.

patrickhulce · 2019-05-30T15:18:09Z

lighthouse-core/report/html/report-styles.css

+}
+.lh-gauge__label-info-icon {
+  pointer-events: fill;
+  cursor: help;


feels a tad odd to me to flip the cursor since there's no actual click action here, is there a broader UX strategy here? I would love to learn more about these kinds of things :)

honestly, I had the bring the cursor back with CSS so I just picked one. Since there's no click action to prompt perhaps it should just remain a normal cursor. IDK.

@yuinchien what do you think of this?
https://lighthouse-33lan68g9.now.sh/#accessibility (hover over i)

@hoten I suggest shifting the position 4px downward and to the right.

I applied transform: translate3d(4px,4px,0) to the info icon and it looks like this.

patrickhulce · 2019-05-30T15:26:48Z

lighthouse-core/test/report/html/renderer/pwa-category-renderer-test.js

@@ -242,7 +242,7 @@ describe('PwaCategoryRenderer', () => {
  describe('#renderScoreGauge', () => {
    it('renders an error score gauge in case of category error', () => {
      category.score = null;
-      const badgeGauge = pwaRenderer.renderScoreGauge(category);
+      const badgeGauge = pwaRenderer.renderScoreGauge(category, null);


what's the point of this one? typedef suggests both are unexpected but looking through the code it looks like we never used groups for the score gauge no matter what.... 🤷‍♂

don't recall, removed.

brendankenny · 2019-06-06T20:19:07Z

https://www.matuzo.at/blog/building-the-most-inaccessible-site-possible-with-a-perfect-lighthouse-score/

is an argument against moving the accessibility description to a tooltip (see especially the conclusion at the end). Maybe it would be better just to shorten + only exist if there's something really important to say

connorjclark · 2019-06-27T23:49:10Z

what are we doing with this?

brendankenny · 2019-07-20T00:10:03Z

what are we doing with this?

I don't think we want to do this. Implementation complexity, not a huge win in terms of simplification, and an arguable net loss of explainable utility

connorjclark added 2 commits May 3, 2019 12:36

report(redesign): tooltip for category description

d8616e6

Merge remote-tracking branch 'origin/master' into rd-cat-desc-tooltip

4f49f3d

connorjclark requested a review from paulirish as a code owner May 3, 2019 19:37

connorjclark commented May 3, 2019

View reviewed changes

align top

6ab11e8

paulirish reviewed May 3, 2019

View reviewed changes

vercel bot deployed to staging May 3, 2019 19:43 View deployment

template, kill animiation delay

481ad34

vercel bot temporarily deployed to staging May 3, 2019 20:04 Inactive

kill max width to fix clipping issue

a51d517

vercel bot deployed to staging May 3, 2019 20:04 View deployment

fix link bullsht

deffec8

vercel bot deployed to staging May 3, 2019 20:22 View deployment

dark

1238796

vercel bot deployed to staging May 3, 2019 20:38 View deployment

paulirish reviewed May 4, 2019

View reviewed changes

paulirish added waiting4committer P1 labels May 5, 2019

paulirish previously approved these changes May 6, 2019

View reviewed changes

fix test by adding a wrapper el

b769c58

vercel bot deployed to staging May 6, 2019 17:36 View deployment

connorjclark commented May 6, 2019

View reviewed changes

paulirish added land-when-ci-is-green and removed waiting4committer labels May 6, 2019

Merge remote-tracking branch 'origin/master' into rd-cat-desc-tooltip

7c7f0f5

vercel bot deployed to staging May 6, 2019 19:07 View deployment

vercel bot deployed to staging May 23, 2019 16:53 View deployment

disable header link

b4d50a7

vercel bot deployed to staging May 23, 2019 19:10 View deployment

connorjclark added the 5.1 label May 28, 2019

fix tooltip and linking

41db4bb

vercel bot deployed to staging May 29, 2019 19:12 View deployment

googlebot added the cla: yes label May 29, 2019

remove opts

f9dfbab

vercel bot deployed to staging May 29, 2019 20:36 View deployment

fix pwa

670df6e

vercel bot deployed to staging May 29, 2019 21:51 View deployment

connorjclark closed this May 30, 2019

connorjclark reopened this May 30, 2019

patrickhulce approved these changes May 30, 2019

View reviewed changes

remove null in test

f4cedf7

vercel bot deployed to staging May 30, 2019 16:55 View deployment

remove ws

02abf1b

vercel bot deployed to staging May 30, 2019 16:56 View deployment

connorjclark removed the 5.1 label May 30, 2019

connorjclark added the waiting4review label Jul 12, 2019

connorjclark closed this Jul 20, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

report(redesign): tooltip for category description #8840

report(redesign): tooltip for category description #8840

connorjclark commented May 3, 2019

connorjclark May 3, 2019

paulirish May 3, 2019

paulirish May 3, 2019

paulirish May 3, 2019

connorjclark May 3, 2019

paulirish May 3, 2019

paulirish commented May 6, 2019

paulirish left a comment

connorjclark commented May 6, 2019

connorjclark May 6, 2019

brendankenny May 6, 2019

connorjclark May 6, 2019

paulirish commented May 6, 2019

connorjclark commented May 6, 2019 via email

connorjclark commented May 23, 2019

connorjclark commented May 25, 2019 •

edited

Loading

brendankenny commented May 29, 2019

connorjclark commented May 29, 2019

patrickhulce commented May 29, 2019

connorjclark commented May 29, 2019

patrickhulce left a comment

patrickhulce May 30, 2019

connorjclark May 30, 2019

patrickhulce May 30, 2019

connorjclark May 30, 2019

patrickhulce May 30, 2019

connorjclark May 30, 2019 •

edited

Loading

yuinchien May 30, 2019

patrickhulce May 30, 2019

connorjclark May 30, 2019

brendankenny commented Jun 6, 2019

connorjclark commented Jun 27, 2019

brendankenny commented Jul 20, 2019

report(redesign): tooltip for category description #8840

report(redesign): tooltip for category description #8840

Conversation

connorjclark commented May 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulirish commented May 6, 2019

paulirish left a comment

Choose a reason for hiding this comment

connorjclark commented May 6, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulirish commented May 6, 2019

connorjclark commented May 6, 2019 via email

connorjclark commented May 23, 2019

connorjclark commented May 25, 2019 • edited Loading

brendankenny commented May 29, 2019

connorjclark commented May 29, 2019

patrickhulce commented May 29, 2019

connorjclark commented May 29, 2019

patrickhulce left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

connorjclark May 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendankenny commented Jun 6, 2019

connorjclark commented Jun 27, 2019

brendankenny commented Jul 20, 2019

connorjclark commented May 25, 2019 •

edited

Loading

connorjclark May 30, 2019 •

edited

Loading