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?
> >
> >
> 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.
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 browser/ views/minimized .js (right):
> > File app/subapps/
> >
> >
https:/ /codereview. appspot. com/13092044/ diff/1/ app/subapps/ browser/ views/minimized .js#newcode52 browser/ views/minimized .js:52: Binds the viewmode
> > app/subapps/
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/