Review: Needs Information =push_breadcrumb=
* Why call "hide" on all breadcrumbs? Why not just on the last one in the series? This requires the (undoc) guarantee that "hide" can be called repeatedly * Related, are views never used in/for breadcrumbs? I don't see any proxy from hide to do_hide. * Why not use _.default rather than repeated "item[key] = item[key] || default"? * Is it really a good idea to alter the argument in-place? Wouldn't it be better to create a new empty object and merge the parameter's stuff into it? * The keys of the object passed as parameters should be doxed. =on_breadcrumb_clicked= * Why not use _.find? * Not big stuff, but why use `i += 1` rather than `i++` (or `++i`)? =remove_breadcrumbs= * Wouldn't the interface be better/simpler if it took a breadcrumb item? (the implementation would be a bit more complex) =get_title= I don't think I understand what it's supposed to do. I understand the toplevel loop but not the inner one… each title can be composed of sub-titles which are separated by "/" but still link to the same item? -- https://code.launchpad.net/~openerp-dev/openerp-web/trunk-breadcrumb-fme/+merge/113432 Your team OpenERP R&D Team is subscribed to branch lp:~openerp-dev/openerp-web/trunk-breadcrumb-fme. _______________________________________________ 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
