-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
core(locales): streamline hover texts #6508
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @midzer appreciate the support !
@@ -12,7 +12,7 @@ const UIStrings = { | |||
/** The name of the metric that marks the estimated time between the page receiving input (a user clicking, tapping, or typing) and the page responding. Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit. */ | |||
title: 'Estimated Input Latency', | |||
/** Description of the Estimated Input Latency metric that estimates the amount of time, in milliseconds, that the app takes to respond to user input. This description is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */ | |||
description: 'The score above is an estimate of how long your app takes to respond to user ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes good catch we should definitely change from The score above
to Estimated Input Latency
👍
I'm not sure I like a rough guess
rather than an estimate
, that feels like it doesn't give us quite enough credit for how it's computed :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, simply estimate
is probably more meaningful. I want to avoid the word twice in a sentence. Maybe there's an alternative word around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm not sure about "rough guess" maybe "an approximation"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 on keeping "estimate"
@@ -12,7 +12,7 @@ const UIStrings = { | |||
/** The name of the metric that marks the time at which the page is fully loaded and is able to quickly respond to user input (clicks, taps, and keypresses feel responsive). Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit. */ | |||
title: 'Time to Interactive', | |||
/** Description of the Time to Interactive (TTI) metric, which evaluates when a page has completed its primary network activity and main thread work. This is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */ | |||
description: 'Interactive marks the time at which the page is fully interactive. ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We went with this because the actual moment is called interactive
and the metric is the timespan until that moment Time To Interactive. Not opposed to rewording so that the name of the metric is the subject but we'll need to change up the rest of the sentence then :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this was my intention to bring the title to first words in the description as everywhere else already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better as "Time to interactive is the amount of time it takes for the page to become fully interactive?" Since yea, there is a unit disconnect between a span of time (tti) and a point in time (interactive).
nit: What is the capitalization: Time to Interactive
or Time to interactive
?
@@ -162,7 +162,7 @@ | |||
"message": "Minimizes main thread work" | |||
}, | |||
"lighthouse-core/audits/metrics/estimated-input-latency.js | description": { | |||
"message": "The score above is an estimate of how long your app takes to respond to user input, in milliseconds, during the busiest 5s window of page load. If your latency is higher than 50 ms, users may perceive your app as laggy. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/estimated-input-latency)." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for helping with the translations! I'm afraid these files don't actually persist since they get automatically wiped out when new messages come in from the translators.
@exterkamp do you know if we have a way of passing along suggestions to translators? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To my knowledge (other than the above line comments), no? At the risk of this becoming a daisy chain of escalation @paulirish?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol these are just the backwards files 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 i18n! Love the i18n polish, have a few comments.
@@ -12,7 +12,7 @@ const UIStrings = { | |||
/** The name of the metric that marks the estimated time between the page receiving input (a user clicking, tapping, or typing) and the page responding. Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit. */ | |||
title: 'Estimated Input Latency', | |||
/** Description of the Estimated Input Latency metric that estimates the amount of time, in milliseconds, that the app takes to respond to user input. This description is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */ | |||
description: 'The score above is an estimate of how long your app takes to respond to user ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm not sure about "rough guess" maybe "an approximation"?
@@ -162,7 +162,7 @@ | |||
"message": "Minimizes main thread work" | |||
}, | |||
"lighthouse-core/audits/metrics/estimated-input-latency.js | description": { | |||
"message": "The score above is an estimate of how long your app takes to respond to user input, in milliseconds, during the busiest 5s window of page load. If your latency is higher than 50 ms, users may perceive your app as laggy. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/estimated-input-latency)." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To my knowledge (other than the above line comments), no? At the risk of this becoming a daisy chain of escalation @paulirish?
@@ -12,7 +12,7 @@ const UIStrings = { | |||
/** The name of the metric that marks the time at which the page is fully loaded and is able to quickly respond to user input (clicks, taps, and keypresses feel responsive). Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit. */ | |||
title: 'Time to Interactive', | |||
/** Description of the Time to Interactive (TTI) metric, which evaluates when a page has completed its primary network activity and main thread work. This is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */ | |||
description: 'Interactive marks the time at which the page is fully interactive. ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better as "Time to interactive is the amount of time it takes for the page to become fully interactive?" Since yea, there is a unit disconnect between a span of time (tti) and a point in time (interactive).
nit: What is the capitalization: Time to Interactive
or Time to interactive
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so it seems like the current status is
- Keep the Estimated Input Latency change but maintain it's an estimate, not just a rough guess
- Possibly update the "Time to Interactive" (that's a phrase people are familiar with), but the rest of the sentence will need to be reworded
- drop the changes to
ar-XB.json
since they won't be able to be used
I've just pushed an update |
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi :)
Summary
Rework minor unbalance on main performance items for hover text
More or less polishing
Let's sync the way hover texts introduce information
More languages need to be checked...
Related Issues/PRs
I've tested latest alpha and stumbled upon this issue