[opensource-dev] Review Request: STORM-1103 Nearby sidebar minimap should be optional

[Boroondas Gupte]

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110414/63f0d2a9/attachment-0001.htm