UI improvements #17
UI improvements #17
Conversation
app/public.js
Outdated
| @@ -259,12 +252,20 @@ | |||
| } | |||
|
|
|||
| function getSize(element) { | |||
| let performanceEntry = performance.getEntriesByName(getUrl(element))[0]; | |||
| performanceEntry = performance.getEntriesByName(getUrl(element))[0]; | |||
There was a problem hiding this comment.
@simonhearne MAINTENANCE - use local variable instead of global use let
app/public.js
Outdated
| if (performanceEntry) { | ||
| return performanceEntry.encodedBodySize; | ||
| } | ||
| } | ||
|
|
||
| function getDuration(element) { | ||
| performanceEntry = performance.getEntriesByName(element)[0]; |
There was a problem hiding this comment.
@simonhearne MAINTENANCE - use local variable instead of global use let
app/public.js
Outdated
| function getDuration(element) { | ||
| performanceEntry = performance.getEntriesByName(element)[0]; | ||
| if (performanceEntry) { | ||
| console.log(performanceEntry); |
There was a problem hiding this comment.
@simonhearne MAINTENANCE - remove all console logs please
app/public.js
Outdated
|
|
||
| function getHost(s) { | ||
| let l = document.createElement('a'); | ||
| l.href=s; |
There was a problem hiding this comment.
@simonhearne MAINTENANCE - use URL api
let url = new URL('http://test.google.com/after?bool')
return url.hostname;
This will exclude ports btw.
app/styles/content.css
Outdated
| @@ -19,6 +19,7 @@ | |||
| z-index: 999999 !important; | |||
| box-sizing: border-box !important; | |||
| padding: 5px !important; | |||
| font-size:90%; | |||
There was a problem hiding this comment.
@simonhearne CONSISTENCY - use consistent style with spaces after :
app/public.js
Outdated
| return image.url; | ||
| } | ||
| function getImageScale(image) { | ||
| let wid = image.naturalSize.width / image.width; |
There was a problem hiding this comment.
@simonhearne MAINTENANCE - use readable variable names width, height
app/public.js
Outdated
| function getImageScale(image) { | ||
| let wid = image.naturalSize.width / image.width; | ||
| let hei = image.naturalSize.height / image.height; | ||
| return Math.max(wid,hei); |
There was a problem hiding this comment.
@simonhearne CONSISTENCY - space after comma
app/public.js
Outdated
| @@ -281,6 +282,20 @@ | |||
| } | |||
| } | |||
|
|
|||
| function getName(s) { | |||
| let n = s.replace(/^.*[\\\/]/, ''); | |||
| if (n.indexOf('?')>0) { | |||
There was a problem hiding this comment.
@simonhearne INFO - are you sure you want to eliminate query strings? some sites use them to fetch different versions of the image.
There was a problem hiding this comment.
This is just to render the filename in the overlay, removing query strings makes the filename more readable and the href is still available for the full image URL.
app/styles/popup.css
Outdated
| } | ||
| button { |
There was a problem hiding this comment.
@simonhearne CONSISTENCY - create a class. Don't target html tags directly.
app/styles/popup.css
Outdated
| @@ -14,6 +14,28 @@ | |||
| limitations under the License. | |||
| */ | |||
|
|
|||
| html, body { | |||
| /*width: 390px;*/ | |||
There was a problem hiding this comment.
@simonhearne MAINTENANCE - remove comment
app/public.js
Outdated
|
|
||
| return COLORS.blue; | ||
| let col = percentage; | ||
| if (percentage < 0) col = 0; |
There was a problem hiding this comment.
@simonhearne CONSISTENCY - always use { for any if statements even if it's a single line
app/popup.html
Outdated
| <button id="stop">Remove Overlay</button> | ||
| </div> | ||
| <div class="popup-contact"> | ||
| <a href="https://www.nccgroup.trust/web-performance/" id="contact">Need help?</a> |
There was a problem hiding this comment.
@simonhearne DoD - add target="_blank" as it doesn't open otherwise
# Conflicts: # app/public.js # test/public.spec.js
…age-checker into simon--popup-ui-improvements
No description provided.