Re: [opensource-dev] Review Request: STORM-1103 Nearby sidebar minimap should be optional
--- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/265/ --- (Updated June 26, 2011, 8:32 a.m.) Review request for Viewer. Changes --- Corrects an issue identified in JIRA STORM-1103 where it is possible to make it so the map can no longer be displayed. Quoted from JIRA 1. Open Nearby Panel 2. Decrease viewer window height until mini-map disappears automatically 3. Disable mini-map in the gear menu 4. Maximize viewer window height 5. Enable minim-map via gear menu Actual Results Minimap isn't displayed without this patch. With this patch, the Minimap is still displayable. 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://bitbucket.org/Twisted_Laws/viewer-development-storm-1103 as https://bitbucket.org/Twisted_Laws/viewer-development-storm-1103/changeset/3455e79a14af and https://bitbucket.org/Twisted_Laws/viewer-development-storm-1103/changeset/48b66643a4d1 This addresses bug STORM-1103. http://jira.secondlife.com/browse/STORM-1103 Diffs (updated) - indra/newview/skins/default/xui/en/panel_people.xml 48b66643a4d1 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
Re: [opensource-dev] Review Request: STORM-1103 Nearby sidebar minimap should be optional
--- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/265/#review726 --- Ship it! work fine on linux, nothing evil visible after some time of usage. - Altair On May 14, 2011, 1:07 p.m., Twisted Laws wrote: --- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/265/ --- (Updated May 14, 2011, 1:07 p.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://bitbucket.org/Twisted_Laws/viewer-development-storm-1103 as https://bitbucket.org/Twisted_Laws/viewer-development-storm-1103/changeset/3455e79a14af and https://bitbucket.org/Twisted_Laws/viewer-development-storm-1103/changeset/48b66643a4d1 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
Re: [opensource-dev] Review Request: STORM-1103 Nearby sidebar minimap should be optional
--- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/265/ --- (Updated May 14, 2011, 1:07 p.m.) Review request for Viewer. Changes --- corrected a bad url to the repository in my description and added the second changeset url Summary (updated) --- 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://bitbucket.org/Twisted_Laws/viewer-development-storm-1103 as https://bitbucket.org/Twisted_Laws/viewer-development-storm-1103/changeset/3455e79a14af and https://bitbucket.org/Twisted_Laws/viewer-development-storm-1103/changeset/48b66643a4d1 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
Re: [opensource-dev] Review Request: STORM-1103 Nearby sidebar minimap should be optional
On April 14, 2011, 1:01 p.m., Vadim ProductEngine wrote: indra/newview/skins/default/xui/en/menu_people_nearby_view_sort.xml, line 45 http://codereview.secondlife.com/r/265/diff/1/?file=1474#file1474line45 I'd say View Mini-Map: by Map we usually mean the world map, which is obviously a different thing. I was making the other changes mentioned in the reviews, but this one I question... imho, putting in View Mini-Map is misleading also as a requirement of implementation was to leave the mini-map available seperately. I would not consider this the mini-map. But I'm open to have it however you (as a Linden) want it. - Twisted --- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/265/#review603 --- 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
Re: [opensource-dev] Review Request: STORM-1103 Nearby sidebar minimap should be optional
On April 14, 2011, 1:01 p.m., Vadim ProductEngine wrote: indra/newview/skins/default/xui/en/menu_people_nearby_view_sort.xml, line 45 http://codereview.secondlife.com/r/265/diff/1/?file=1474#file1474line45 I'd say View Mini-Map: by Map we usually mean the world map, which is obviously a different thing. Twisted Laws wrote: I was making the other changes mentioned in the reviews, but this one I question... imho, putting in View Mini-Map is misleading also as a requirement of implementation was to leave the mini-map available seperately. I would not consider this the mini-map. But I'm open to have it however you (as a Linden) want it. I'm not insisting, that's just my opinion. Let's leave it for PO consideration. - Vadim --- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/265/#review603 --- 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
Re: [opensource-dev] Review Request: STORM-1103 Nearby sidebar minimap should be optional
--- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/265/ --- (Updated April 15, 2011, 11:46 a.m.) Review request for Viewer. Changes --- revised patch is attached, repository updated 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 (updated) - 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
Re: [opensource-dev] Review Request: STORM-1103 Nearby sidebar minimap should be optional
--- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/265/#review621 --- Ship it! indra/newview/skins/default/xui/en/panel_people.xml http://codereview.secondlife.com/r/265/#comment590 Put a space before the / and it's perfect, I'd say. (I cannot tell whether all the numbers for the positioning attributes make sense, but I guess you would have noticed while testing if not.) - Boroondas On April 15, 2011, 11:46 a.m., Twisted Laws wrote: --- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/265/ --- (Updated April 15, 2011, 11:46 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
[opensource-dev] Review Request: STORM-1103 Nearby sidebar minimap should be optional
--- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/265/ --- 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
Re: [opensource-dev] Review Request: STORM-1103 Nearby sidebar minimap should be optional
--- 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 stringShow/hide additional mini-map above nearby list/string or stringShow/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
Re: [opensource-dev] Review Request: STORM-1103 Nearby sidebar minimap should be optional
On April 14, 2011, 12:37 p.m., Boroondas Gupte wrote: indra/newview/skins/default/xui/en/panel_people.xml, lines 117-127 http://codereview.secondlife.com/r/265/diff/1/?file=1475#file1475line117 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. I always wondered about the sequencing of these, but I figured that it was some application that made it easier to work on these did that ordering. The code that reads it doesn't care about ordering. As far as if fields are required or not, I've no idea as I don't believe I have any access to any documentation. Me working on XML files is try it, change it, try it, etc... I'll leave it sit a little while for any other comments, and then I'll change it as you suggest. - Twisted --- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/265/#review602 --- 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
Re: [opensource-dev] Review Request: STORM-1103 Nearby sidebar minimap should be optional
--- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/265/#review603 --- Ship it! Looks fine and works well for me, however I don't quite understand why to limit your changes with XML files. Resizing the mini-map might improve usability. indra/newview/app_settings/settings.xml http://codereview.secondlife.com/r/265/#comment565 I'd say nearby people list, not nearby list. indra/newview/skins/default/xui/en/menu_people_nearby_view_sort.xml http://codereview.secondlife.com/r/265/#comment568 I'd say View Mini-Map: by Map we usually mean the world map, which is obviously a different thing. indra/newview/skins/default/xui/en/panel_people.xml http://codereview.secondlife.com/r/265/#comment566 Reformat with alphabetic parameter order and place one parameter per line. That's more diff-friendly and also improves compatibility with the XUI preview tool. indra/newview/skins/default/xui/en/panel_people.xml http://codereview.secondlife.com/r/265/#comment569 This looks like irrelevant change. - Vadim 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
Re: [opensource-dev] Review Request: STORM-1103 Nearby sidebar minimap should be optional
On April 14, 2011, 12:37 p.m., Boroondas Gupte wrote: indra/newview/skins/default/xui/en/panel_people.xml, lines 117-127 http://codereview.secondlife.com/r/265/diff/1/?file=1475#file1475line117 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. Twisted Laws wrote: I always wondered about the sequencing of these, but I figured that it was some application that made it easier to work on these did that ordering. The code that reads it doesn't care about ordering. As far as if fields are required or not, I've no idea as I don't believe I have any access to any documentation. Me working on XML files is try it, change it, try it, etc... I'll leave it sit a little while for any other comments, and then I'll change it as you suggest. The application is the XUI preview tool (Ctrl+T in the login screen). It dumps XMLs with attributes sorted by name. Please do so as well. - Vadim --- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/265/#review602 --- 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
