Repository navigation
Convert to ES6 Modules - #19
Conversation
|
@rniwa I missed your formatting CL. I'll wait for that one to land first. |
Just for reference, can you link to the source you pulled in for this file? |
juliandescottes
left a comment
juliandescottes
left a comment
There was a problem hiding this comment.
Thanks, overall this looks good to me. There are remaining references to the "logo-link" id to cleanup (I assume this breaks displaying the results?)
|
I trust Julian's reviews, so removing myself |
|
|
||
| img { | ||
| user-select: none; | ||
| -webkit-user-select: none; |
There was a problem hiding this comment.
user-select seems to not work as expected in Safari 16.0, thus I left this in.
|
PTAL again |
| document.getElementById('screen-size-warning').style.display = screenIsTooSmall ? null : 'none'; | ||
| } | ||
|
|
||
| _startBenchmarkHandler() { |
There was a problem hiding this comment.
I think we should just call this startBenchmarkHandler. _ prefix doesn't really buy us anything.
There was a problem hiding this comment.
Agreed there is no real "protected". Since we do have other _-prefixed methods in the MainBenchmarkClient, how about following one of the following strategies for more consistency:
a) Consistently use _-prefix for "protected" methods that are not referenced from outside the class
b) Drop _ prefix alltogether
- add showAbout method
- Rename files from .js to .mjs (not strictly necessary but it makes clear that we use modules) - Pull in missing statistics.js file from https://github.com/WebKit/WebKit/blob/main/PerformanceTests/resources/statistics.js - Restructure UI init code to be all in main.mjs - Moved free-floating UI code to MainBenchmarkClient - Added `_isRunning` instance variable to MainBenchmarkClient - Fix CSS to address removed links - Use `<button>` instead of links for better keyboard navigation in all browsers
Address PR comments.
_isRunninginstance variable to MainBenchmarkClient<button>instead of links for better keyboard navigation