Review: Needs Fixing
* Some of the behaviors (e.g. `$('#oe_app_search table:last').css('display',
'none');`) are not clear and pretty reckless: if you go into Accounting >
Customer Payments for instance, the two fields on the second line (Journal and
Period) completely disappear when you click "clear"
* Also what's with the weird rule in search.js:327? I really don't think that's
correct, you can have groups defaulting to expanded at pretty much every
location. For instance, Sales > Reporting > Phone Calls Analysis. The
`group_by` group defaults to expanded, clicking `clear` folds it which makes
the view harder to use (it's an analysis view, so it never displays the records
themselves only grouping results)
* Indentation is fucked up, please fix
* Please scope all DOM searches to this.$element, don't do global ones. Also
*definitely* don't hardcode ids like `oe_app_search`, that's not correct and
it's going to break if a searchview is set in a different WebClient instance
not called `oe_app`.
* Usage of `.empty()` is good, usage of `.css('display', 'none')` is not.
Please use `.hide()`
* Likewise, use `toggleClass` to toggle classes on and off, you can give it
multiple classes (see search.js:418)
* I'm quite uncertain about the looping on `:input` elements: since we're not
cancelling the form's default `on_clear` behavior, all normal fields get
cleared either way, if you remove the loop and try clearing the form you should
see regular <input> elements and selections still clear themselves correctly
* The clearing sequence seems broken in some edge cases, please investigate: if
you go to Sales > Configuration > Sales > Invoice Types, give a value to Active
and Invoicing Method selections and search you get no result (so far so good).
Now if you click `clear` you *still* get no result, if you click `search`
explicitly after that you do get the two original results. You should get two
results after clearing only once. This might be because the default clear
action (from html forms) executes after the event handler, therefore the search
runs before the form is fully cleared.
* Maybe it would be a good idea to implement a clearing protocol, similar to
the get_domain and get_context stuff where each search widget (including
extended search) can add special behavior if they need to (e.g. if they're more
complex widgets than a simple input, which gets cleared by the form either
way), so you don't create kinda-sorta broken rules? This way e.g. groups could
override the default clearing action (which might be to do nothing at all) in
order to re-set their state to the original one.
--
https://code.launchpad.net/~openerp-dev/openerp-web/search-clear-kbh-vja/+merge/70425
Your team OpenERP R&D Team is subscribed to branch
lp:~openerp-dev/openerp-web/search-clear-kbh-vja.
_______________________________________________
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
