Tree refactoring, plugin api changes #457
Conversation
baev
commented
Jul 4, 2017
baev
commented
- Sign CLA
- Add unit tests
| @@ -71,7 +71,8 @@ class WidgetsGridView extends View { | |||
|
|
|||
| this.addRegion(name, {el: el.find('.widget__body')}); | |||
| /// fetchAndShow(this, name, this.model, new Widget({model: this.model})) | |||
| this.getRegion(name).show(new Widget({model: this.model})); | |||
| const model = this.model; | |||
| this.getRegion(name).show(new Widget({model, name})); | |||
There was a problem hiding this comment.
would be better to prepare the correct model here
new Widget({model: this.model.getWidgetData(name)})
instead of doing this inside the Widget constructor
There was a problem hiding this comment.
the problem is that graphs are widgets as well
There was a problem hiding this comment.
You mean this: https://github.com/allure-framework/allure2/blob/master/allure-generator/src/main/javascript/plugins/widget-status/StatusWidgetView.js
So, what is the problem?
There was a problem hiding this comment.
I mean that graphs tab uses WidgetsGridView but model is different (plain list of results)
| statistic[value] = 0; | ||
| }); | ||
| items.forEach(item => { | ||
| if (item.children) { |
There was a problem hiding this comment.
you can also add a check for item.statistic and don't do it on every forEach loop
| return time; | ||
| } | ||
|
|
||
| static updateTime(timeA, timeB, field, operation) { |
There was a problem hiding this comment.
This is not Java you don't need to use static methods, you can just create a standalone function within this module
| } else if ('status' in a && 'status' in b) { | ||
| return nodeCmp(a, b); | ||
| function compare(a, b, nodeCmp, groupCmp, direction) { | ||
| if (a.hasOwnProperty('children') && !b.hasOwnProperty('children')) { |
There was a problem hiding this comment.
why do you do this instead of just if (a.children && !b.children) {?
|
Also tested manually, key navigation was not broken 👍 |
sseliverstov
force-pushed
the
tree-refactoring
branch
from
72012ed
to
a14464c
Compare
