yellow team mailing list archive

[Matthew Scott <matthew.scott@xxxxxxxxxxxxx>]

We discussed the lack of scroll-wheel capability on the call (something
I apparently relied on quite a bit, judging by how I did my functional
testing - perhaps a relatively high priority on that).

However, another regression that I noticed in testing was the fact that
menus are not positioned properly, nor do they follow the service
they're attached to when zooming or panning; they remain at (0,0).
uistage shows the service menu remaining open and following the service
during both pan and zoom events, no matter the source.

I'm not comfortable having two user-visible regressions in one branch -
I'm fine with the scroll-wheel one because that one isn't necessarily
discoverable, though it's certainly high on my own personal list, but I
think this second one is a little too visibly not right.  Just one minor
in code, so far, though I'll give it a more thorough looking-over later
today (it's hard to tell all that changed with these refactors).

Thanks for the branch!


https://codereview.appspot.com/7071045/diff/9001/app/assets/javascripts/d3-components.js
File app/assets/javascripts/d3-components.js (right):

https://codereview.appspot.com/7071045/diff/9001/app/assets/javascripts/d3-components.js#newcode203
app/assets/javascripts/d3-components.js:203: //console.debug('Handler
for', name, selector, d3.event);
Could be removed or just uncommented, unless it causes too much clutter
in the console.

https://codereview.appspot.com/7071045/diff/9001/app/views/environment.js
File app/views/environment.js (right):

https://codereview.appspot.com/7071045/diff/9001/app/views/environment.js#newcode71
app/views/environment.js:71: rendered: function() {
Good rename, I think.

https://codereview.appspot.com/7071045/diff/9001/app/views/topology/viewport.js
File app/views/topology/viewport.js (right):

https://codereview.appspot.com/7071045/diff/9001/app/views/topology/viewport.js#newcode43
app/views/topology/viewport.js:43: resized: function() {
Overall a great simplification.  I like it!

https://codereview.appspot.com/7071045/

-- 
https://code.launchpad.net/~bcsaller/juju-gui/viewport/+merge/142138
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~bcsaller/juju-gui/viewport into lp:juju-gui.