Prevent duplicate infobox fetches
Review Request #8568 — Created Dec. 12, 2016 and submitted
Previously, datagrids would attach additional event handler for
infoboxes on top of the initial event handler. This would result in two
fetches for all infoboxes in the datagrid until the first refresh. Now
we set an attribute on elements which can have infoboxes attached them
when the infobox handler has been added so that it will not be added
again.
Verified infoboxes in datagrids are no longer requested twice.
| Description | From | Last Updated |
|---|---|---|
|
Can we make this data-has-${id}-infobox? As-is it'll make attributes like data-has-user which isn't immediately obvious. |
 david
|
|
|
Early returns increase complexity. Given that the code that would be skipped by it is only three statements, how about … |
david
|
|
|
This needs to be $target now, because fat-arrow functions have lexical this binding. |
david
|
|
|
We should use .data() instead for storing this, rather than using an attribute. We actually can attach the view as … |
 chipx86
|
-
-
reviewboard/static/rb/js/common.es6.js (Diff revision 1) Can we make this
data-has-${id}-infobox? As-is it'll make attributes likedata-has-userwhich isn't immediately obvious. -
reviewboard/static/rb/js/common.es6.js (Diff revision 1) Early returns increase complexity. Given that the code that would be skipped by it is only three statements, how about we instead do:
if (!$target.attr(infoboxAttr)) { const view = new RB.InfoboxView({ $target: $target, el: $el, }); view.render(); $target.attr(infoboxAttr, true); } -
reviewboard/static/rb/js/common.es6.js (Diff revision 1) This needs to be
$targetnow, because fat-arrow functions have lexicalthisbinding.
Change Summary:
Addressed David's issues.
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 2 (+13 -6) |
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/common.es6.js Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/common.es6.js
-
-
reviewboard/static/rb/js/common.es6.js (Diff revision 2) We should use
.data()instead for storing this, rather than using an attribute.We actually can attach the view as the data. This is what jQuery widgets do. This allows for fetching the view for a given element as well.
(
.data()doesn't simply wrap.attr('data-...'))
Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/common.es6.js Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/common.es6.js