Re: [opensource-dev] Review Request: STORM-1567 Mute button for llDialog popup

2011-08-26 Thread Vadim ProductEngine 

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



indra/newview/lltoastnotifypanel.cpp


The surrounding code looks quite messy (not your fault, I know), so please 
make more comments to the code you add.



indra/newview/lltoastnotifypanel.cpp


I didn't go through the calculations, but the Mute button is visually 
bigger than the Ignore button. Is it intentional?



indra/newview/llviewermessage.cpp


It's a hack to update the Block List panel from here.
The panel should be an observer of the mute list, or something like that, 
i.e. update itself automatically.



indra/newview/skins/default/xui/en/notifications.xml


I suppose these changes must be also made to the "ScriptDialog" 
notification template.


- Vadim


On Aug. 23, 2011, 1:41 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/449/
> ---
> 
> (Updated Aug. 23, 2011, 1:41 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Add a block button to popups from llDialog calls.  Clicking on the Block 
> button adds the object to the block list.  The type of block is Object.  
> Changing the object's name does not defeat this block as the block entry is 
> stored as a UUID.
> 
> Note: The object's name in the block list does not change if the object is 
> renamed, though the block does work.  This may need to be addressed in a 
> separate SVC jira once this change is incorporated into viewer-development.
> 
> 
> This addresses bug STORM-1567.
> http://jira.secondlife.com/browse/STORM-1567
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 4ebbd04efd93 
>   indra/newview/lltoastnotifypanel.cpp 4ebbd04efd93 
>   indra/newview/llviewermessage.cpp 4ebbd04efd93 
>   indra/newview/skins/default/xui/en/notifications.xml 4ebbd04efd93 
> 
> Diff: http://codereview.secondlife.com/r/449/diff
> 
> 
> Testing
> ---
> 
> Tested per Test Plan jira entry.
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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-638 "Object Return" doesn't return distant objects

2011-08-26 Thread Vadim ProductEngine 

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

Review request for Viewer.


Summary
---

Reason: Showing the confirmation dialog resets object selection,
thus there is nothing to derez.

Fix: Save selection until user answers in the confirmation dialog.

I didn't investigate why the bug occurred only for distant object
(must be some internal LLSelectMgr magic).


This addresses bug STORM-638.
http://jira.secondlife.com/browse/STORM-638


Diffs
-

  indra/newview/llviewermenu.cpp 64ed6f9362af 

Diff: http://codereview.secondlife.com/r/451/diff


Testing
---


Thanks,

Vadim

___
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-638 "Object Return" doesn't return distant objects

2011-08-26 Thread Vadim ProductEngine 

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

(Updated Aug. 26, 2011, 8:38 a.m.)


Review request for Viewer and Seth ProductEngine.


Summary
---

Reason: Showing the confirmation dialog resets object selection,
thus there is nothing to derez.

Fix: Save selection until user answers in the confirmation dialog.

I didn't investigate why the bug occurred only for distant object
(must be some internal LLSelectMgr magic).


This addresses bug STORM-638.
http://jira.secondlife.com/browse/STORM-638


Diffs
-

  indra/newview/llviewermenu.cpp 64ed6f9362af 

Diff: http://codereview.secondlife.com/r/451/diff


Testing (updated)
---

See acceptance criteria in the JIRA ticket.


Thanks,

Vadim

___
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-1567 Mute button for llDialog popup

2011-08-26 Thread Jonathan Yap 


> On Aug. 26, 2011, 6:05 a.m., Vadim ProductEngine wrote:
> > indra/newview/lltoastnotifypanel.cpp, line 339
> > 
> >
> > The surrounding code looks quite messy (not your fault, I know), so 
> > please make more comments to the code you add.

Looking over this mathematical mess I found that ignore_btn_hoffset was a 
leftover from when I was developing this change and was no longer needed.

I have added one comment.


> On Aug. 26, 2011, 6:05 a.m., Vadim ProductEngine wrote:
> > indra/newview/lltoastnotifypanel.cpp, lines 359-369
> > 
> >
> > I didn't go through the calculations, but the Mute button is visually 
> > bigger than the Ignore button. Is it intentional?

Yes the block button is larger than the ignore button because:
1) It is the same size as is found in other instances in the UI where it 
appears.
2) I did not know if making it smaller would affect the width needed for 
language translations.


> On Aug. 26, 2011, 6:05 a.m., Vadim ProductEngine wrote:
> > indra/newview/llviewermessage.cpp, lines 6556-6559
> > 
> >
> > It's a hack to update the Block List panel from here.
> > The panel should be an observer of the mute list, or something like 
> > that, i.e. update itself automatically.

This exact same call is used in 6 other places in the code.  It forces the 
sidebar open and highlights the newly Blocked item as feedback to the user that 
their Block click has had some effect, so I think having this call is correct.


> On Aug. 26, 2011, 6:05 a.m., Vadim ProductEngine wrote:
> > indra/newview/skins/default/xui/en/notifications.xml, lines 6279-6282
> > 
> >
> > I suppose these changes must be also made to the "ScriptDialog" 
> > notification template.

Good catch!


- Jonathan


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


On Aug. 23, 2011, 1:41 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/449/
> ---
> 
> (Updated Aug. 23, 2011, 1:41 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Add a block button to popups from llDialog calls.  Clicking on the Block 
> button adds the object to the block list.  The type of block is Object.  
> Changing the object's name does not defeat this block as the block entry is 
> stored as a UUID.
> 
> Note: The object's name in the block list does not change if the object is 
> renamed, though the block does work.  This may need to be addressed in a 
> separate SVC jira once this change is incorporated into viewer-development.
> 
> 
> This addresses bug STORM-1567.
> http://jira.secondlife.com/browse/STORM-1567
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 4ebbd04efd93 
>   indra/newview/lltoastnotifypanel.cpp 4ebbd04efd93 
>   indra/newview/llviewermessage.cpp 4ebbd04efd93 
>   indra/newview/skins/default/xui/en/notifications.xml 4ebbd04efd93 
> 
> Diff: http://codereview.secondlife.com/r/449/diff
> 
> 
> Testing
> ---
> 
> Tested per Test Plan jira entry.
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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-1567 Mute button for llDialog popup

2011-08-26 Thread Vadim ProductEngine 


> On Aug. 26, 2011, 6:05 a.m., Vadim ProductEngine wrote:
> > indra/newview/llviewermessage.cpp, lines 6556-6559
> > 
> >
> > It's a hack to update the Block List panel from here.
> > The panel should be an observer of the mute list, or something like 
> > that, i.e. update itself automatically.
> 
> Jonathan Yap wrote:
> This exact same call is used in 6 other places in the code.  It forces 
> the sidebar open and highlights the newly Blocked item as feedback to the 
> user that their Block click has had some effect, so I think having this call 
> is correct.

Ah, right. You see -- more comments won't hurt! :-)


- Vadim


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


On Aug. 23, 2011, 1:41 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/449/
> ---
> 
> (Updated Aug. 23, 2011, 1:41 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Add a block button to popups from llDialog calls.  Clicking on the Block 
> button adds the object to the block list.  The type of block is Object.  
> Changing the object's name does not defeat this block as the block entry is 
> stored as a UUID.
> 
> Note: The object's name in the block list does not change if the object is 
> renamed, though the block does work.  This may need to be addressed in a 
> separate SVC jira once this change is incorporated into viewer-development.
> 
> 
> This addresses bug STORM-1567.
> http://jira.secondlife.com/browse/STORM-1567
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 4ebbd04efd93 
>   indra/newview/lltoastnotifypanel.cpp 4ebbd04efd93 
>   indra/newview/llviewermessage.cpp 4ebbd04efd93 
>   indra/newview/skins/default/xui/en/notifications.xml 4ebbd04efd93 
> 
> Diff: http://codereview.secondlife.com/r/449/diff
> 
> 
> Testing
> ---
> 
> Tested per Test Plan jira entry.
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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-1567 Mute button for llDialog popup

2011-08-26 Thread Jonathan Yap 

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

(Updated Aug. 26, 2011, 9:37 a.m.)


Review request for Viewer.


Changes
---

New diff to reflect changes made from RB comments.


Summary
---

Add a block button to popups from llDialog calls.  Clicking on the Block button 
adds the object to the block list.  The type of block is Object.  Changing the 
object's name does not defeat this block as the block entry is stored as a UUID.

Note: The object's name in the block list does not change if the object is 
renamed, though the block does work.  This may need to be addressed in a 
separate SVC jira once this change is incorporated into viewer-development.


This addresses bug STORM-1567.
http://jira.secondlife.com/browse/STORM-1567


Diffs (updated)
-

  doc/contributions.txt 4ebbd04efd93 
  indra/newview/lltoastnotifypanel.cpp 4ebbd04efd93 
  indra/newview/llviewermessage.cpp 4ebbd04efd93 
  indra/newview/skins/default/xui/en/notifications.xml 4ebbd04efd93 

Diff: http://codereview.secondlife.com/r/449/diff


Testing
---

Tested per Test Plan jira entry.


Thanks,

Jonathan

___
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] [JIRA] Proposal - Default Search settings on jira.secondlife.com

2011-08-26 Thread opensourceobscure 
Currently, if you search multiple keywords on jira.secondlife.com
you get result that include at least one of the keywords
in Summary OR Description OR Comments fields.

Unsurprisingly you often get thousands of results for every search,
which is pointless. This requires users to tweak the search
parameters, which is not trivial for common people.

Instead, common people may know (by using Google) that
the more keywords you use, the best results you get.
On SL JIRA, it's the opposite.


PROPOSAL:

1) exclude "Comments" from fields where keywords are searched for
2) exclude "Description" too (this may be excessive but usually works for me)
3) replace the "OR" logic operator (which is implicit in searches)
with the "AND" logic operator

I think that this would lead to a better use of JIRA for
SL users willing to give feedback. More specifically,
the number of duplicate issues may be reduced because
the search for existing issues would be more efficient.

-- 
Opensource Obscure
https://twitter.com/oobscure
https://my.secondlife.com/opensource.obscure
Join this group to discuss Second Life Viewer: https://j.mp/slv2group
___
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] [JIRA] Proposal - Default Search settings on jira.secondlife.com

2011-08-26 Thread Robert Martin 
On Fri, Aug 26, 2011 at 1:34 PM, opensourceobscure
 wrote:
> Currently, if you search multiple keywords on jira.secondlife.com
> you get result that include at least one of the keywords
> in Summary OR Description OR Comments fields.
>
actually i would like to see ALL KEYWORDS set as default on all LL
searches since if i search for say
|Girls Ballet Costume| in the MarketPlace
i would like to see items with ALL of those keywords not just some of them

(and there are still keyword stuffed entries in the marketplace it
would be nice if those got sorted out)
-- 
Robert L Martin
___
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-638 "Object Return" doesn't return distant objects

2011-08-26 Thread Richard Nelson 

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



indra/newview/llviewermenu.cpp


This is dangerous!  Never hold on to raw pointers to llviewerobject, as 
they can be deleted from under you.  Use LLPointer instead.

I looked into fixing the underlying problem, but that involves revisiting 
the design of selections.  We always deselect objects beyond a certain distance 
in order to keep your avatar proximate to the objects you can affect (for 
various technical and social reasons).  This workaround is fine for now, if not 
ideal.


- Richard


On Aug. 26, 2011, 8:38 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/451/
> ---
> 
> (Updated Aug. 26, 2011, 8:38 a.m.)
> 
> 
> Review request for Viewer and Seth ProductEngine.
> 
> 
> Summary
> ---
> 
> Reason: Showing the confirmation dialog resets object selection,
> thus there is nothing to derez.
> 
> Fix: Save selection until user answers in the confirmation dialog.
> 
> I didn't investigate why the bug occurred only for distant object
> (must be some internal LLSelectMgr magic).
> 
> 
> This addresses bug STORM-638.
> http://jira.secondlife.com/browse/STORM-638
> 
> 
> Diffs
> -
> 
>   indra/newview/llviewermenu.cpp 64ed6f9362af 
> 
> Diff: http://codereview.secondlife.com/r/451/diff
> 
> 
> Testing
> ---
> 
> See acceptance criteria in the JIRA ticket.
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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-638 "Object Return" doesn't return distant objects

2011-08-26 Thread Richard Nelson 

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


do not ship until fixing the pointer problem

- Richard


On Aug. 26, 2011, 8:38 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/451/
> ---
> 
> (Updated Aug. 26, 2011, 8:38 a.m.)
> 
> 
> Review request for Viewer and Seth ProductEngine.
> 
> 
> Summary
> ---
> 
> Reason: Showing the confirmation dialog resets object selection,
> thus there is nothing to derez.
> 
> Fix: Save selection until user answers in the confirmation dialog.
> 
> I didn't investigate why the bug occurred only for distant object
> (must be some internal LLSelectMgr magic).
> 
> 
> This addresses bug STORM-638.
> http://jira.secondlife.com/browse/STORM-638
> 
> 
> Diffs
> -
> 
>   indra/newview/llviewermenu.cpp 64ed6f9362af 
> 
> Diff: http://codereview.secondlife.com/r/451/diff
> 
> 
> Testing
> ---
> 
> See acceptance criteria in the JIRA ticket.
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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] [JIRA] Proposal - Default Search settings on jira.secondlife.com

2011-08-26 Thread Dave Booth 
Opinions inline...

On 8/26/2011 12:34 PM, opensourceobscure wrote:

> PROPOSAL:
>
> 1) exclude "Comments" from fields where keywords are searched for

Agree

> 2) exclude "Description" too (this may be excessive but usually works for me)

Disagree on the grounds that excluding this one would negatively impact 
one category of benefits you hope to achieve, namely the reduction of 
duplicate issues. Users tend to express the same issue in different 
words when dealing with two similar fields on the same form. Searching 
both makes the target of the search "wider" in that the original 
reporter has expressed it two ways and the search (reflecting the way 
the SEARCHER thinks of the issue) is more likely to hit one.

> 3) replace the "OR" logic operator (which is implicit in searches)
> with the "AND" logic operator

Oh heck, YES. Include the "quoted multiword string as single search 
token" functionality too.

>
> I think that this would lead to a better use of JIRA for
> SL users willing to give feedback. More specifically,
> the number of duplicate issues may be reduced because
> the search for existing issues would be more efficient.
>

___
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-638 "Object Return" doesn't return distant objects

2011-08-26 Thread Vadim ProductEngine 


> On Aug. 26, 2011, 11:51 a.m., Richard Nelson wrote:
> > indra/newview/llviewermenu.cpp, line 4267
> > 
> >
> > This is dangerous!  Never hold on to raw pointers to llviewerobject, as 
> > they can be deleted from under you.  Use LLPointer instead.
> > 
> > I looked into fixing the underlying problem, but that involves 
> > revisiting the design of selections.  We always deselect objects beyond a 
> > certain distance in order to keep your avatar proximate to the objects you 
> > can affect (for various technical and social reasons).  This workaround is 
> > fine for now, if not ideal.

Ah, crap. I was going to use an LLPointer but forgot then. Thank you very much 
for spotting, will fix!


- Vadim


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


On Aug. 26, 2011, 8:38 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/451/
> ---
> 
> (Updated Aug. 26, 2011, 8:38 a.m.)
> 
> 
> Review request for Viewer and Seth ProductEngine.
> 
> 
> Summary
> ---
> 
> Reason: Showing the confirmation dialog resets object selection,
> thus there is nothing to derez.
> 
> Fix: Save selection until user answers in the confirmation dialog.
> 
> I didn't investigate why the bug occurred only for distant object
> (must be some internal LLSelectMgr magic).
> 
> 
> This addresses bug STORM-638.
> http://jira.secondlife.com/browse/STORM-638
> 
> 
> Diffs
> -
> 
>   indra/newview/llviewermenu.cpp 64ed6f9362af 
> 
> Diff: http://codereview.secondlife.com/r/451/diff
> 
> 
> Testing
> ---
> 
> See acceptance criteria in the JIRA ticket.
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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