Code review comment for lp://staging/~rockstar/loggerhead/unify-templates

Martin Albisetti wrote:
> On Wed, Aug 13, 2008 at 12:26 AM, Tim Penhey <email address hidden> wrote:
>
>>> === added file 'loggerhead/templates/feed-link.pt'
>>> --- loggerhead/templates/feed-link.pt 1970-01-01 00:00:00 +0000
>>> +++ loggerhead/templates/feed-link.pt 2008-08-13 02:25:09 +0000
>>> @@ -0,0 +1,8 @@
>>> +<tal:feed-link>
>>> + <div>
>>> + <a tal:attributes="href python:url(['/atom']);
>>> + title string:RSS feed for ${branch/friendly_name}">
>>> + <img tal:attributes="src python:branch.static_url('/static/images/ico_rss.gif')" alt="RSS" class="rssfeed"/>
>>> + </a>
>>> + </div>
>>> +</tal:feed-link>
>>>
>> Do we really want to have the feed-link in a div all the time?
>> Perhaps it would be better to wrap it in a div outside of this page template.
>>
>
> Outside?
> I don't quite follow what you mean by that.
>
>
I think he means back into macros.pt I can do this easily, and I agree.
>>> === added file 'loggerhead/templates/menu.pt'
>>> --- loggerhead/templates/menu.pt 1970-01-01 00:00:00 +0000
>>> +++ loggerhead/templates/menu.pt 2008-08-13 02:25:09 +0000
>>> @@ -0,0 +1,27 @@
>>> +<tal:menu>
>>> + <ul id="menuTabs">
>>> + <form tal:attributes="action python:branch.url('/changes', start_revid=getattr(navigation, 'start_revid', None),
>>> + file_id=getattr(navigation, 'file_id', None))">
>>> + <div id="finderBox">
>>> + <tal:search-box replace="structure python:search_box()" />
>>> + <tal:feed-link replace="structure python:feed_link(branch)" />
>>> + </div>
>>> + </form>
>>>
>> By putting these in the normal menu page template, doesn't that make it
>> harder to break them out and put them somewhere else in the page?
>>
> I agree, search can/should be separate.
> Can you do this with the current HTML or do you need me to tweak it a little?
>
I think we'll need to tweak it, but with some guidance, we can probably
do it in the same branch.
>> Is there any way we can add extra tabs? Like in the GNOME theme?
>>
> More <li>'s should do it. Any reason why not?
>
I think that's where you go to edit the menu template itself.
>> :-( I find myself wanting a "Tweak" vote.
>>
> BB has set high standards for all of us :)
>
John Arbash Meinel made the same comment when reviewing my branch for
bzr-stats

--
Paul Hummer
Launchpad Code Team