> =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
Right. Fixed! > * Related, are views never used in/for breadcrumbs? I don't see any proxy > from hide to do_hide. Breadcrumb is not view aware, neither view manager aware. It should work with any widget so it does not directly deal with views, but you can find breadcrumb usage customization in ViewManager#add_breadcrumb > * Why not use _.default rather than repeated "item[key] = item[key] || > default"? done > * 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? dunno, is it a problem ? > * The keys of the object passed as parameters should be doxed. Ok, done. I'm not a tox of dox :-) > =on_breadcrumb_clicked= > > * Why not use _.find? Because the loop serves a second purpose not possible to bundle in a _.find > * Not big stuff, but why use `i += 1` rather than `i++` (or `++i`)? It's my javascript snippets engine for loops, is it a problem ? > =remove_breadcrumbs= > > * Wouldn't the interface be better/simpler if it took a breadcrumb item? (the > implementation would be a bit more complex) Not sure, in fact, internal refactoring probably shows an oddity. There used to be a pop()/back() and other methods acting like an array, this is why I worked with indexes. Now that those functions are removed, maybe it looks odd. Do you want remove_breadcrumb to support both an index or an item object ? > =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? yes. This is because the breadcrumb is not specifically designed for view managers. If a widget should display more than one title (eg: view managers) then it get_title should return an array of titles, show() will receive the index of the title selected by the user. -- 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
