Code review comment for lp:~jcsackett/juju-gui/browse-will-not-open-from-minimized

Revision history for this message
Richard Harding (rharding) wrote :

On 2013/08/21 12:57:03, j.c.sackett wrote:
> On 2013/08/20 20:44:46, jeff.pihach wrote:
> > LGTM however I'm a little concerned about the duplication of code -
possibly
> > create a follow-up to normalize this stuff?
> >
> >

https://codereview.appspot.com/13092044/diff/1/app/subapps/browser/views/minimized.js
> > File app/subapps/browser/views/minimized.js (right):
> >
> >

https://codereview.appspot.com/13092044/diff/1/app/subapps/browser/views/minimized.js#newcode52
> > app/subapps/browser/views/minimized.js:52: Binds the viewmode
control events
> to
> > navigation.
> > This code appears to be duplicated in other parts of the application
- can it
> > not be split out into a mixin and then mixed into all classes which
need it?

> I agree it should be deduped, but the best way to do that is actually
to just
> have minimized also use MainView; since MainView sets up all the
search stuff,
> this should probably be done when we do the card "Make search bar stay
when
> sidebar is minimized". I'll add a comment to that effect.

I was wondering if the viewmode-controls widget module could provide a
view extension that could be used to provide the setup/bind event
helpers for Views to use? I thought we had done this with another module
before.

https://codereview.appspot.com/13092044/

« Back to merge proposal