-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/265/#review602
-----------------------------------------------------------

Ship it!


Already giving a 'Ship it', as all my comments below address very minor stuff.


indra/newview/app_settings/settings.xml
<http://codereview.secondlife.com/r/265/#comment564>

    <nitpick>
    Technically, this doesn't show the (main) Map, but an (additional) 
Mini-Map. And that isn't in the nearby people list, but above it.
    </nitpick>
    
    So maybe make this
          <string>Show/hide additional mini-map above nearby list</string>
    or
          <string>Show/hide additional mini-map in 'Nearby People' 
sidepanel</string>
    
    (... or not. Once people try it, it should be clear enough. And hopefully 
most will prefer using the gear menu over altering the debug setting manually.)



indra/newview/skins/default/xui/en/panel_people.xml
<http://codereview.secondlife.com/r/265/#comment562>

    Are the 'top' and 'left' attributes really needed? I guess they default to 
0 anyway when omitted ...



indra/newview/skins/default/xui/en/panel_people.xml
<http://codereview.secondlife.com/r/265/#comment559>

    <net_map /> is inside <layout_panel />, so it should be indented one level 
more.



indra/newview/skins/default/xui/en/panel_people.xml
<http://codereview.secondlife.com/r/265/#comment560>

    <avatar_list /> is inside <layout_panel />, so it should be indented one 
level more.



indra/newview/skins/default/xui/en/panel_people.xml
<http://codereview.secondlife.com/r/265/#comment561>

    I'd prefer to have the attributes ordered semantically (i.e. 'name' first, 
'top' and 'left' right after each other, 'height' and 'width' right after each 
other etc.) rather than alphabetically. But as the surrounding code also seems 
to have its attributes ordered alphabetically, we might as well stick to that. 
Though, then, keep_one_selected should be moved up.


- Boroondas


On April 14, 2011, 5:29 a.m., Twisted Laws wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/265/
> -----------------------------------------------------------
> 
> (Updated April 14, 2011, 5:29 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> Patch makes the map in the Nearby people tab optional with a menu option in 
> the gear 
> menu.  Patch is XML only and resizing of the map is disabled 
> (user_resize="false" in 
> the layout_panels) as I could not find a way to easily save window sizes 
> purely in XML.
> Patch is in the repository of 
> https://Twisted_Laws/viewer-development-storm-1103 as
> https://bitbucket.org/Twisted_Laws/viewer-development-storm-1103/changeset/3455e79a14af
> 
> 
> This addresses bug STORM-1103.
>     http://jira.secondlife.com/browse/STORM-1103
> 
> 
> Diffs
> -----
> 
>   doc/contributions.txt ee4d271eef9b 
>   indra/newview/app_settings/settings.xml ee4d271eef9b 
>   indra/newview/skins/default/xui/en/menu_people_nearby_view_sort.xml 
> ee4d271eef9b 
>   indra/newview/skins/default/xui/en/panel_people.xml ee4d271eef9b 
> 
> Diff: http://codereview.secondlife.com/r/265/diff
> 
> 
> Testing
> -------
> 
> Tested by exercising the gear menu option of "View Map" with the People tab 
> attached 
> and detached insuring the map appears and disappears properly.
> 
> 
> Thanks,
> 
> Twisted
> 
>

_______________________________________________
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Reply via email to