i18n: rewrite dom-size description to be more i18n friendly
#9690
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
exterkamp
requested review from
brendankenny,
patrickhulce and
paulirish
as code owners
brendankenny
approved these changes
Sep 18, 2019
| @@ -27,8 +27,8 @@ const UIStrings = { | |||
| failureTitle: 'Avoid an excessive DOM size', | |||
| /** Description of a Lighthouse audit that tells the user *why* they should reduce the size of the web page's DOM. The size of a DOM is characterized by the total number of DOM elements and greatest DOM depth. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */ | |||
| description: 'Browser engineers recommend pages contain fewer than ' + | |||
There was a problem hiding this comment.
Suggested change
| description: 'Browser engineers recommend pages contain fewer than ' + | |
| description: 'Browser engineers recommend that pages contain fewer than ' + |
There was a problem hiding this comment.
(I'm not sure why the diff is showing currently three spaces and this is deleting one when it's clearly going 2->2, but I guess you can't just accept these changes cause it would need collect-strings anyway :)
| `~${MAX_DOM_ELEMENTS.toLocaleString()} DOM elements. The sweet spot is a tree ` + | ||
| `depth < ${MAX_DOM_TREE_DEPTH} elements and fewer than ${MAX_DOM_TREE_WIDTH} ` + | ||
| `~${MAX_DOM_ELEMENTS.toLocaleString()} DOM elements. A more ideal layout is a tree ` + | ||
| `depth less than ${MAX_DOM_TREE_DEPTH} elements and fewer than ${MAX_DOM_TREE_WIDTH} ` + | ||
| 'children/parent element. A large DOM can increase memory usage, cause longer ' + |
There was a problem hiding this comment.
Suggested change
| 'children/parent element. A large DOM can increase memory usage, cause longer ' + | |
| 'children per parent. A large DOM can increase memory usage, cause longer ' + |
paulirish
approved these changes
Sep 18, 2019
| @@ -27,8 +27,8 @@ const UIStrings = { | |||
| failureTitle: 'Avoid an excessive DOM size', | |||
| /** Description of a Lighthouse audit that tells the user *why* they should reduce the size of the web page's DOM. The size of a DOM is characterized by the total number of DOM elements and greatest DOM depth. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */ | |||
| description: 'Browser engineers recommend pages contain fewer than ' + | |||
| `~${MAX_DOM_ELEMENTS.toLocaleString()} DOM elements. The sweet spot is a tree ` + | |||
| `depth < ${MAX_DOM_TREE_DEPTH} elements and fewer than ${MAX_DOM_TREE_WIDTH} ` + | |||
| `~${MAX_DOM_ELEMENTS.toLocaleString()} DOM elements. A more ideal layout is a tree ` + | |||
There was a problem hiding this comment.
i dont like using the term layout like this.
Suggested change
| `~${MAX_DOM_ELEMENTS.toLocaleString()} DOM elements. A more ideal layout is a tree ` + | |
| `~${MAX_DOM_ELEMENTS.toLocaleString()} DOM elements. Ideally, the DOM tree's` + |
| `~${MAX_DOM_ELEMENTS.toLocaleString()} DOM elements. The sweet spot is a tree ` + | ||
| `depth < ${MAX_DOM_TREE_DEPTH} elements and fewer than ${MAX_DOM_TREE_WIDTH} ` + | ||
| `~${MAX_DOM_ELEMENTS.toLocaleString()} DOM elements. A more ideal layout is a tree ` + | ||
| `depth less than ${MAX_DOM_TREE_DEPTH} elements and fewer than ${MAX_DOM_TREE_WIDTH} ` + |
There was a problem hiding this comment.
Suggested change
| `depth less than ${MAX_DOM_TREE_DEPTH} elements and fewer than ${MAX_DOM_TREE_WIDTH} ` + | |
| ` maximum depth is less than ${MAX_DOM_TREE_DEPTH} elements and each parent element has fewer than ${MAX_DOM_TREE_WIDTH} ` + |
| `~${MAX_DOM_ELEMENTS.toLocaleString()} DOM elements. The sweet spot is a tree ` + | ||
| `depth < ${MAX_DOM_TREE_DEPTH} elements and fewer than ${MAX_DOM_TREE_WIDTH} ` + | ||
| `~${MAX_DOM_ELEMENTS.toLocaleString()} DOM elements. A more ideal layout is a tree ` + | ||
| `depth less than ${MAX_DOM_TREE_DEPTH} elements and fewer than ${MAX_DOM_TREE_WIDTH} ` + | ||
| 'children/parent element. A large DOM can increase memory usage, cause longer ' + |
There was a problem hiding this comment.
100% agree with brendan's edit, but this would be edit given my rewording.
Suggested change
| 'children/parent element. A large DOM can increase memory usage, cause longer ' + | |
| 'children. A large DOM can increase memory usage, cause longer ' + |
There was a problem hiding this comment.
FWIW in #9725 I also just proposed nuking these specific numbers entirely if that somehow helps the i18nness at the same time given our mismatch to scoring :)
+1 I don't think that these numbers are really as concrete as we present. Would be nice to remove/have new numbers/have them confirmed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
tc/ came back with a concern about the language in
dom-size, namely about "sweet spot" being toen-UScentric.