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. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
