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(audit): make dom-size table prettier #6065
Merged
Merged
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
3a69f6d
Made the dom-size table more readable.
exterkamp 276a5e9
Moved attribute pruning code to page-functions. Added unit tests to …
exterkamp 244496e
i18n for new table. Removed some comments.
exterkamp c184bb6
Refactored getOuterHTMLSnippet to use removeAttribute.
exterkamp 9aaa581
Changed up UIStrings comment wording.
exterkamp 077eb85
Changed wording in comments + docstrings. Changed testing to use dire…
exterkamp 71a2b18
Changed UI Texts as per Paul's comments. Cleaned up page-functions-t…
exterkamp File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
/** | ||
* @license Copyright 2018 Google Inc. All Rights Reserved. | ||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. | ||
*/ | ||
'use strict'; | ||
|
||
const assert = require('assert'); | ||
const jsdom = require('jsdom'); | ||
const DOM = require('../../report/html/renderer/dom.js'); | ||
const pageFunctions = require('../../lib/page-functions'); | ||
|
||
/* eslint-env jest */ | ||
|
||
describe('DetailsRenderer', () => { | ||
let dom; | ||
|
||
beforeAll(() => { | ||
const document = jsdom.jsdom(); | ||
dom = new DOM(document); | ||
}); | ||
|
||
afterAll(() => { | ||
exterkamp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
global.URL = undefined; | ||
global.Util = undefined; | ||
}); | ||
|
||
describe('get outer HTML snippets', () => { | ||
it('gets full HTML snippet', () => { | ||
assert.equal(pageFunctions.getOuterHTMLSnippet( | ||
dom.createElement('div', '', {id: '1', style: 'style'})), '<div id="1" style="style">'); | ||
}); | ||
|
||
it('removes a specific attribute', () => { | ||
assert.equal(pageFunctions.getOuterHTMLSnippet( | ||
dom.createElement('div', '', {id: '1', style: 'style'}), ['style']), '<div id="1">'); | ||
}); | ||
|
||
it('removes multiple attributes', () => { | ||
assert.equal(pageFunctions.getOuterHTMLSnippet( | ||
dom.createElement('div', '', {'id': '1', 'style': 'style', 'aria-label': 'label'}), | ||
['style', 'aria-label'] | ||
), '<div id="1">'); | ||
}); | ||
|
||
it('ignores when attribute not found', () => { | ||
assert.equal(pageFunctions.getOuterHTMLSnippet( | ||
dom.createElement('div', '', {'id': '1', 'style': 'style', 'aria-label': 'label'}), | ||
['style-missing', 'aria-label-missing'] | ||
), '<div id="1" style="style" aria-label="label">'); | ||
}); | ||
}); | ||
}); |
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
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.
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.
bikeshed. wdyt about 'Statistic' or 'Metric'. cc @brendankenny @patrickhulce
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.
I like either of those, maybe Metric more? Agreed they're slightly more descriptive than
Category
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.
I took the Category name from the
Minimizes main thread work
Audit table. So we can update the phrasing in every table if we like Metric more (which I agree is more descriptive than Category).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.
heh, in that case I feel like "Category" is better (since it's actually bringing e.g. multiple Script Evaluation tasks together and grouping them into one line), but also see the appeal of consistency