> > return this.views[view_type] && this.views[view_type].view_id || false;
>
> Why not use a standard C-style ternary for that?
>
> return this.views[view_type] ? this.views[view_type].view_id : false;
>
> completely standard and more readable no?
because even if this.views[view_type] exists, it's view_id might be `undefined`
(in case of ViewManage - not ViewManagerAction)
> > title: _t("Create: ") + ' ' + title,
>
> I think it'd be better (and simpler for translators) to use a translatable
> pattern for this one e.g.
>
> _.str.sprintf(_t("Create: %s"), title)
>
> also not sure why there is a space *in* the translatable string and one more
> outside of it. Same for "Edit" below, except it does not have the extra space
Those translation were as is before my patch, I hesitated to do so but I was
not sure if we could change the translations on 7.0 so I left them as they
were. Do you confirm we can change translations on 7.0 ?
> Finally, the chunks of code in slow_create and open_event seem to be exactly
> the same, why not extract in a method returning an object with 2 keys?
I hesitated for this too. Going to do it right now.
--
https://code.launchpad.net/~openerp-dev/openerp-web/7.0-fix-calendar-formpopup-view/+merge/146401
Your team OpenERP R&D Team is subscribed to branch
lp:~openerp-dev/openerp-web/7.0-fix-calendar-formpopup-view.
_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-gtk
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openerp-dev-gtk
More help : https://help.launchpad.net/ListHelp
