refactor tabs and radio stuff out #69
Conversation
| panelsContent = tabs.map(t => t.content); | ||
| } else { | ||
| panelsContent = tabs.map(t => renderTabPanel(t)); | ||
| } |
There was a problem hiding this comment.
Slight bug fix here -- if there's only 1 browser showing we don't render the tabs at all, so this also skips rendering the surrounding panel.
| @@ -464,8 +330,7 @@ class Test extends LitElement { | |||
| } | |||
|
|
|||
| } | |||
| _renderTestResults(browser, testData) { | |||
| const result = testData.results.find(r => r.name === browser); | |||
| _renderTestResults(result) { | |||
There was a problem hiding this comment.
Seems like this is maybe overkill now, and the div could just be added to _renderTestResult?
There was a problem hiding this comment.
I guess there's a lot of early returns in there. Could maybe move this up into:
content: html`<div class="result">this._renderTestResult(result)</div>`,
Either way, minor, just an idea
There was a problem hiding this comment.
Yeah I had started leaving space for when this is actually going to be rendering multiple test results -- i.e. once you can click on an entire file and see all its tests. Definitely overkill right now though.
dlockhart
force-pushed
the
dlockhart/refactor-report-part1
branch
from
296322f
to
c12cca4
Compare
|
🎉 This PR is included in version 0.15.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
ghost
added
the
No visual or functional changes here.
This moves all the tabs and pillbox related rendering and CSS out into separate helper files. This will help reduce the size of
test.jsin preparation for merging it back intoapp.jsin part 2 of the refactoring. Because Rollup builds all this together, there's no impact on the report output files -- still justindex.htmlandapp.js.Background: I originally split these into separate components but in since they don't really need to be reused they're creating overhead to pass stuff down and communicate up.