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.
_______________________________________________
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
