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

clients(viewer): add disclaimer regarding devtools bug #12846

Merged
merged 3 commits into from
Aug 3, 2021

Conversation

connorjclark
Copy link
Collaborator

See #12841

@connorjclark connorjclark requested a review from a team as a code owner August 2, 2021 22:38
@connorjclark connorjclark requested review from patrickhulce and removed request for a team August 2, 2021 22:38
@google-cla google-cla bot added the cla: yes label Aug 2, 2021
lighthouse-viewer/app/index.html Outdated Show resolved Hide resolved
@@ -35,6 +35,11 @@ <h1 class="viewer-placeholder__heading">Lighthouse Report Viewer</h1>
or <a href="https://web.dev/measure">web.dev/measure</a>.</div>
<input type="url" class="viewer-placeholder__url js-gist-url"
placeholder="Enter or paste Gist URL">

<br>
<b>DevTools users:</b> there is a <a href="https://github.com/GoogleChrome/lighthouse/issues/12841">known bug</a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

No interest in the "no query param, no referrer" to narrow this to more likely devtools users? it's a lot of (somewhat embarrassing) text to show to the definitely uninterested majority

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

definitely uninterested majority

I disagree that this wouldn't be useful to the majority of traffic to this page.

You can ignore the traffic coming via the extension, because the entire page is blurred while loading. Same for gist/json. So you're left with people who go directly to the viewer, which will be a small fraction compared to the traffic that will come from DevTools soon (although, this remains to be seen).

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can ignore the traffic coming via the extension, because the entire page is blurred while loading. Same for gist/json.

This is the traffic for whom I'm saying we should hide this message :)

When something fails with the fetch for the 99.9% of users that definitely did not come from DevTools, seeing a message about DevTools that occupies over half the explanatory text feels unnecessary, that's all.

lighthouse-viewer/app/index.html Outdated Show resolved Hide resolved
@paulirish
Copy link
Member

lgtm.

can you remove the font-weight style in the .viewer-placeholder rule as a driveby? i'm looking at https://lighthouse-4dzl8hfjk-googlechrome.vercel.app/gh-pages/viewer/ but the text is so damn thin its hard to read.

@patrickhulce
Copy link
Collaborator

LGTM, thanks! :)

@connorjclark connorjclark merged commit 067dc60 into master Aug 3, 2021
@connorjclark connorjclark deleted the viewer-dt-disclaimer branch August 3, 2021 21:56
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.

4 participants