Re: [review-request] bbarrosp/all-pages

[Michael Wood <michael.g.wood@intel.com>]

On 13/11/14 11:40, Barros Pena, Belen wrote:
> This branch:
>
> * Fixes the layout of the dependencies modal dialog
> * Removes the external link icon from the target name column when the cell
> is empty
> * Adds the 'no results' state to the all layers page
>
> Thanks!
>
> Belén
>
commit e8107bbb44afeec888667f5239afa33bf71235b4

<td class="target">
                  {{o.name}}
-                <a target="_blank" 
href="{{o.get_layersource_view_url}}"><i class="icon-share 
get-info"></i></a>
+                {% if o.name %}
+                    <a target="_blank" 
href="{{o.get_layersource_view_url}}"><i class="icon-share 
get-info"></i></a>
+                {% endif%}
          </td>

A tiny tiny niggle would be that {{o.name}} could be inside the if 
statement so that when you read it, it's not "use this var...  now check 
it exists" the templating engine will do the right thing so it's just a 
reading issue.

commit 8c38eec9e48e2bcf492584f70991056a3513c479

IMO we should avoid using 'Named access on the Window object' as they're 
a bit dodgy/too magic[1]. But as you know other templates in toaster 
already have this same code, so we may as commit it and fix it with a 
next refactoring commit.

other than that looks good to me.

Michael

[1] good article about that here 
http://tjvantoll.com/2012/07/19/dom-element-references-as-global-variables/