Review: Needs Fixing
* Don't use IDs, there's a risk it's going to break with multiple lists present
on the screen (and in any case it's not semantically correct).
* The code is short and not expected to be overridden (I'd think), so I'd
suggest keeping it inline, not split as a separate method.
* Could you try using .prop() instead of .attr? I'm pretty sure it'd work just
as well, and that's what it's for (technically, we're manipulating the
"checked" attribute)
* Your setting of the check state looks very debatable, from what I understand
it's going to set all checkboxes to the opposite of what the first one is,
irrespective of what the checkbox which was just clicked is. So if you check
the header checkbox, then uncheck the first row's checkbox, and uncheck the
header checkbox… it's going to keep everything checked (and re-check the first
row).
* I don't see the point of using `:checkbox` in your selector (in
`this.$element.find('.oe-record-selector :checkbox')`): there's only ever a
single input (a checkbox) in `.oe-record-selector`, so `:checkbox` does not
avoid selecting anything which should not be selected, it just makes the code
slower.
--
https://code.launchpad.net/~openerp-dev/openerp-web/trunk-bug-878108-kbh/+merge/80188
Your team OpenERP R&D Team is subscribed to branch
lp:~openerp-dev/openerp-web/trunk-bug-878108-kbh.
_______________________________________________
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
