openerp-dev-web team mailing list archive

["Xavier \(Open ERP\)" <xmo@xxxxxxxxxxx>]

Review: Needs Fixing
Review as of revision 3425:

* Missing images (Warehouse, Point of Sale, Tools)
  - Point of Sale had an icon, doesn't seem to be implemented
  - Remove "Inventory" (doesn't exist), use its icon for Warehouse
  - Remove "Chat" icon from CSS, there is no chat toplevel application.
  - apr sent icon for Tools (applications-other.png)
* Font on buttons is too small, and they don't size correctly
  as per previous review, remove subtext in gray and make font bigger
* Also bigger font for applications names
* Homepage generates the following errors in Webkit console:
  <div> is not allowed inside <tr>. Inserting <div> before the <table> instead.
  <td> is not allowed inside <div>. Moving <td> into the nearest enclosing <tabe>.
  <table> is not allowed inside <tbody>. Closing the current <table> and inserting the new <table> as a sibling.
  <div> is not allowed inside <tbody>. Inserting <div> before the <table> instead.
* in index() when calling menu() use keyword arguments: 
    return self.menu(next=next)
  instead of
    return self.menu(None, next)
* I think menu() can be simplified and custom_action has no reason to be in the application URLs, see if this patch seems to work correctly:


=== modified file 'addons/openerp/controllers/root.py'
--- addons/openerp/controllers/root.py	2010-10-04 13:43:28 +0000
+++ addons/openerp/controllers/root.py	2010-10-05 10:07:55 +0000
@@ -104,22 +104,17 @@
         proxy = rpc.RPCProxy("ir.ui.menu")
         ids = proxy.search([('parent_id', '=', False)], 0, 0, 0, ctx)
         parents = proxy.read(ids, ['name', 'action'], ctx)
-        show_tools = False    
-        if not id and ids:
-            id = ids[0]
-            
+        show_tools = False
+
+        if next:
+            show_tools = True
         for parent in parents:
             if parent['id'] == id:
+                show_tools = True
                 parent['active'] = 'active'
-                    
-            if parent.get('action'):
-                parent['url'] = url('/openerp/menu', active=parent['id'], next=url('/openerp/custom_action', action=parent['id']))
-            else:
-                parent['url'] = url('/openerp/menu', active=parent['id'])
-                if parent['id'] == id:
-                    show_tools = True
-        if next:
-            show_tools = True
+                if parent.get('action') and not next:
+                    next = url('/openerp/custom_action', action=parent['id'])
+
         tools = []
         if show_tools: 
             ids = proxy.search([('parent_id', '=', id)], 0, 0, 0, ctx)

=== modified file 'addons/openerp/controllers/templates/index.mako'
--- addons/openerp/controllers/templates/index.mako	2010-10-04 13:43:28 +0000
+++ addons/openerp/controllers/templates/index.mako	2010-10-05 09:59:35 +0000
@@ -36,7 +36,7 @@
                         <ul>
                             %for parent in parents:
                                 <li>
-                                    <a href="${parent['url']}"
+                                    <a href="${py.url('/openerp/menu', active=parent['id'])}"
                                        target="_top" class="${parent.get('active', '')}">
                                         <span>${parent['name']}</span>
                                     </a>
@@ -87,7 +87,7 @@
                                             %for parent in parents:
                                                 <li class="${'-'.join(parent['name'].split(' ')).lower()}">
                                                     <span class="wrap">
-                                                        <a href="${parent['url']}" target="_top">
+                                                        <a href="${py.url('/openerp/menu', active=parent['id'])}" target="_top">
                                                             <span>
                                                                 <strong>${parent['name']}</strong>
                                                             </span>

* Also for clarity's sake, result "proxy" form ``rpc.RPCProxy("ir.ui.menu")``could be called e.g. ``menus`` in order to clearly say *what* it's a proxy to. ``proxy`` is about not-informative as can be.
-- 
https://code.launchpad.net/~openerp-dev/openobject-client-web/web-dashboard/+merge/37436
Your team OpenERP R&D Team is subscribed to branch lp:~openerp-dev/openobject-client-web/web-dashboard.