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

Revision history for this message
j.c.sackett (jcsackett) 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.

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

« Back to merge proposal