Notes from my general review:
The heart isn't centred(design review should cover any remaining problems
- ~~Don't show the survey when the beta signup form is visible
- The beta signup form isn't uniquely identified ~~ (thx @bmansurov)
No browser tests
- No unit tests
QuickSurveys.hooks.php No validation No class that models an internal/external survey No factory that creates the appropriate class from configuration
More sophisticated hooks can be broken out
Addressed in T110196.
Using jQuery to construct a DOM element that might not be necessary(only does it when survey available) Rightwards drift We hit local storage before testing two constants Undocumented, untested insertion point selectionsee https://gerrit.wikimedia.org/r/236040
- [ASSUMPTION] Is the ext.quicksurveys.views really that big that it's worth deferring it? [Discussing in T110200]
- How does VE construct complex layouts? With Mustache?
- The QuickSurvey.widget method is generic when it needn't be -- it's only used for the panels
- The QuickSurvey.log method shouldn't be on a view
- What does a view do?