a little jumble

Hi all, Yana,

This is a series of little modifications which touch several
parts of 
code. I want be sure there isn't
objections before commiting it. I will explain things in the
order they 
appear in the patch file.

first, I added a method
    public Contact getDefaultContact(Class operationSet);
in the metacontact interface and it's implementation based
on
    public Contact getDefaultContact();

The purpose is to obtain a contact for a specific operation.
For example, if
we right click on a metacontact and we choose send file, we
can simply
send the file to the contact returned by
   
metacontact.getDefaultContact(OperationSetFileTransfer.class
);

In ImageLoader.java, I added a static field
public static final ImageLoader.ImageID CALL_16x16_ICON
            = new
ImageID("SIP_COMMUNICATOR_LOGO");
the image is temporary set to SIP_COMMUNICATOR_LOGO.
It is used later for the right button menu over a
metacontact where I 
added a call menu item.
Yana : I see that there is already some icons which could
fit here. 
Could you
help me to pick one ?

ChatSendPanel.java
   fix a bug discovered long time ago with Emil and Yana
which cause
exceptions when there isn't sufficient place in the status
label
(where typing notification are shown) to fit the status
message.

ChatContactPanel.java
    deactivate the action icon on top of the user name which
appears on 
the right
side of a chat window if action is not supported.

ContactRightButtonMenu.java
    add a call menu item

ContactList.java
    popup the right menu at the right coordinates.
Currently, it popups 
only at the good
height.

CallManager.java
    private Contact getTelephonyContact(MetaContact
metaContact)
now returns
metaContact.getDefaultContact(OperationSetBasicTelephony.cla
ss);
rather than the first found telephony contact.
Yana : I think if you agree with this point, the method
could be 
suppressed in favor of
a direct call to 
metaContact.getDefaultContact(OperationSetBasicTelephony.cla
ss);
wherever it is needed.

added method
    public void createCall(Contact contact)

CallPanel.java
    added a new constructor
    public CallPanel(CallManager callManager, Contact
contact)

NightlyBuildID.java : replaced "CVS" by
"Sam" as it more kind
    well, will remove it before committing 
    This said, it should be SVN, isn't it ?

OperationSetMultiUserChatJabberImpl.java
        public ChatRoom findRoom(String roomName)
returns createLocalChatRoomInstance if the room searched
for
doesn't exist, but, in
TestOperationSetMultiUserChatJabberImpl,
 findRoom is supposed to return null in case of an
inexisting room. I 
assumed that
the test expectation was the right behaviour. Isn't it ?

OperationSetBasicTelephonyJabberImpl.java
    cleanup, replaced the null stun server with
"stun.iptel.org".
The stun server provided here is temporary but, isn't better
to have
a valid server (even if the remote machine is down) rather
than using
a null one ? And I think anything is better than null for
good.

Well that's all. These modifications aren't all related, but
as they 
aren't substancial,
I preferred to put them all in a mail. If someone want to
discuss only a 
particular
point there isn't any obligation to care about others 

Regards
    Sympho.





------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

Hi Sympho,

see inline.

Sympho wrote:
> Hi all, Yana,
> 
> This is a series of little modifications which touch
several parts of 
> code. I want be sure there isn't
> objections before commiting it. I will explain things
in the order they 
> appear in the patch file.
> 
> first, I added a method
>    public Contact getDefaultContact(Class
operationSet);
> in the metacontact interface and it's implementation
based on
>    public Contact getDefaultContact();
> 
> The purpose is to obtain a contact for a specific
operation. For 
> example, if
> we right click on a metacontact and we choose send
file, we can simply
> send the file to the contact returned by
>   
metacontact.getDefaultContact(OperationSetFileTransfer.class
);

Great.

> 
> In ImageLoader.java, I added a static field
> public static final ImageLoader.ImageID
CALL_16x16_ICON
>            = new
ImageID("SIP_COMMUNICATOR_LOGO");
> the image is temporary set to SIP_COMMUNICATOR_LOGO.
> It is used later for the right button menu over a
metacontact where I 
> added a call menu item.
> Yana : I see that there is already some icons which
could fit here. 
> Could you
> help me to pick one ?

May be the most appropriate icon is the one used for sip
online status, 
but don't use it from its current location 
(PROJECT_DIR/resources/images/sip), instead you could copy
it in the 
gui/images/common/ under another more appropriate name. Like
this I 
could change it later.

> 
> ChatSendPanel.java
>   fix a bug discovered long time ago with Emil and Yana
which cause
> exceptions when there isn't sufficient place in the
status label
> (where typing notification are shown) to fit the status
message.

Ok.

> ChatContactPanel.java
>    deactivate the action icon on top of the user name
which appears on 
> the right
> side of a chat window if action is not supported.

Great.

> ContactRightButtonMenu.java
>    add a call menu item

Ok.

> 
> ContactList.java
>    popup the right menu at the right coordinates.
Currently, it popups 
> only at the good
> height.

Actually this is not a bug, I thought it's looking better
like this. 
Please wait till I give it one more try before
committing

> 
> CallManager.java
>    private Contact getTelephonyContact(MetaContact
metaContact)
> now returns 
>
metaContact.getDefaultContact(OperationSetBasicTelephony.cla
ss);
> rather than the first found telephony contact.
> Yana : I think if you agree with this point, the method
could be 
> suppressed in favor of
> a direct call to 
>
metaContact.getDefaultContact(OperationSetBasicTelephony.cla
ss);
> wherever it is needed.

I agree.

> 
> added method
>    public void createCall(Contact contact)

Actually there's already a method createCall(Vector
contacts), which is 
meant to be used for selected contacts. The idea is to have
the 
possibility of a multiple selection, which creates a
conference call. I 
think it should be used even if we have only one selected
contact. 
However it could be changed to private and maybe instead of
Vector it's 
better to pass a List.

> 
> CallPanel.java
>    added a new constructor
>    public CallPanel(CallManager callManager, Contact
contact)

The same. There's already a constructor, which takes a list
of contacts.

> 
> NightlyBuildID.java : replaced "CVS" by
"Sam" as it more kind
>    well, will remove it before committing 
>    This said, it should be SVN, isn't it ?

I don't see where is the CVS thing

> 
> OperationSetMultiUserChatJabberImpl.java
>        public ChatRoom findRoom(String roomName)
> returns createLocalChatRoomInstance if the room
searched for
> doesn't exist, but, in
TestOperationSetMultiUserChatJabberImpl,
> findRoom is supposed to return null in case of an
inexisting room. I 
> assumed that
> the test expectation was the right behaviour. Isn't it
?

Actually it's not so simple. The findRoom is meant to search
for a room 
on the server and not only in the local rooms cache. You
could see that
createLocalChatRoomInstance takes a MultiUserChat instance,
so if 
there's no such room on the server it will throw an 
OperationFailedException, when trying to create the multi
user chat.

> 
> OperationSetBasicTelephonyJabberImpl.java
>    cleanup, replaced the null stun server with
"stun.iptel.org".
> The stun server provided here is temporary but, isn't
better to have
> a valid server (even if the remote machine is down)
rather than using
> a null one ? And I think anything is better than null
for good.

I'm not a specialist in this, but it sounds me right

Thanks Sympho!

You have done a really good work!

Yana

> 
> Well that's all. These modifications aren't all
related, but as they 
> aren't substancial,
> I preferred to put them all in a mail. If someone want
to discuss only a 
> particular
> point there isn't any obligation to care about others

> 
> Regards
>    Sympho.
> 
> 
> 
> 
> 
>
------------------------------------------------------------
------------
> 
>
------------------------------------------------------------
---------
> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

Hey Sympho,

When I wrote my mail I didn't see that you've already
committed it all. 
When you are asking for confirmation, which is a very good
idea when 
you're changing someone else's code, you could also wait for
a response

Could you please have a look at my comments and you should
may be turn 
back to the previous version for some of the changes you've
made. (For 
example the OperationSetMultiUserChatJabberImpl should
definitely be 
turned).

Thanks,
Yana

Yana Stamcheva wrote:
> Hi Sympho,
> 
> see inline.
> 
> Sympho wrote:
>> Hi all, Yana,
>>
>> This is a series of little modifications which
touch several parts of 
>> code. I want be sure there isn't
>> objections before commiting it. I will explain
things in the order 
>> they appear in the patch file.
>>
>> first, I added a method
>>    public Contact getDefaultContact(Class
operationSet);
>> in the metacontact interface and it's
implementation based on
>>    public Contact getDefaultContact();
>>
>> The purpose is to obtain a contact for a specific
operation. For 
>> example, if
>> we right click on a metacontact and we choose send
file, we can simply
>> send the file to the contact returned by
>>   
metacontact.getDefaultContact(OperationSetFileTransfer.class
);
> 
> Great.
> 
>>
>> In ImageLoader.java, I added a static field
>> public static final ImageLoader.ImageID
CALL_16x16_ICON
>>            = new
ImageID("SIP_COMMUNICATOR_LOGO");
>> the image is temporary set to
SIP_COMMUNICATOR_LOGO.
>> It is used later for the right button menu over a
metacontact where I 
>> added a call menu item.
>> Yana : I see that there is already some icons which
could fit here. 
>> Could you
>> help me to pick one ?
> 
> May be the most appropriate icon is the one used for
sip online status, 
> but don't use it from its current location 
> (PROJECT_DIR/resources/images/sip), instead you could
copy it in the 
> gui/images/common/ under another more appropriate name.
Like this I 
> could change it later.
> 
>>
>> ChatSendPanel.java
>>   fix a bug discovered long time ago with Emil and
Yana which cause
>> exceptions when there isn't sufficient place in the
status label
>> (where typing notification are shown) to fit the
status message.
> 
> Ok.
> 
>> ChatContactPanel.java
>>    deactivate the action icon on top of the user
name which appears on 
>> the right
>> side of a chat window if action is not supported.
> 
> Great.
> 
>> ContactRightButtonMenu.java
>>    add a call menu item
> 
> Ok.
> 
>>
>> ContactList.java
>>    popup the right menu at the right coordinates.
Currently, it popups 
>> only at the good
>> height.
> 
> Actually this is not a bug, I thought it's looking
better like this. 
> Please wait till I give it one more try before
committing
> 
>>
>> CallManager.java
>>    private Contact getTelephonyContact(MetaContact
metaContact)
>> now returns 
>>
metaContact.getDefaultContact(OperationSetBasicTelephony.cla
ss);
>> rather than the first found telephony contact.
>> Yana : I think if you agree with this point, the
method could be 
>> suppressed in favor of
>> a direct call to 
>>
metaContact.getDefaultContact(OperationSetBasicTelephony.cla
ss);
>> wherever it is needed.
> 
> I agree.
> 
>>
>> added method
>>    public void createCall(Contact contact)
> 
> Actually there's already a method createCall(Vector
contacts), which is 
> meant to be used for selected contacts. The idea is to
have the 
> possibility of a multiple selection, which creates a
conference call. I 
> think it should be used even if we have only one
selected contact. 
> However it could be changed to private and maybe
instead of Vector it's 
> better to pass a List.
> 
>>
>> CallPanel.java
>>    added a new constructor
>>    public CallPanel(CallManager callManager,
Contact contact)
> 
> The same. There's already a constructor, which takes a
list of contacts.
> 
>>
>> NightlyBuildID.java : replaced "CVS" by
"Sam" as it more kind
>>    well, will remove it before committing 
>>    This said, it should be SVN, isn't it ?
> 
> I don't see where is the CVS thing
> 
>>
>> OperationSetMultiUserChatJabberImpl.java
>>        public ChatRoom findRoom(String roomName)
>> returns createLocalChatRoomInstance if the room
searched for
>> doesn't exist, but, in
TestOperationSetMultiUserChatJabberImpl,
>> findRoom is supposed to return null in case of an
inexisting room. I 
>> assumed that
>> the test expectation was the right behaviour. Isn't
it ?
> 
> Actually it's not so simple. The findRoom is meant to
search for a room 
> on the server and not only in the local rooms cache.
You could see that
> createLocalChatRoomInstance takes a MultiUserChat
instance, so if 
> there's no such room on the server it will throw an 
> OperationFailedException, when trying to create the
multi user chat.
> 
>>
>> OperationSetBasicTelephonyJabberImpl.java
>>    cleanup, replaced the null stun server with
"stun.iptel.org".
>> The stun server provided here is temporary but,
isn't better to have
>> a valid server (even if the remote machine is down)
rather than using
>> a null one ? And I think anything is better than
null for good.
> 
> I'm not a specialist in this, but it sounds me
right
> 
> Thanks Sympho!
> 
> You have done a really good work!
> 
> Yana
> 
>>
>> Well that's all. These modifications aren't all
related, but as they 
>> aren't substancial,
>> I preferred to put them all in a mail. If someone
want to discuss only 
>> a particular
>> point there isn't any obligation to care about
others 
>>
>> Regards
>>    Sympho.
>>
>>
>>
>>
>>
>>
------------------------------------------------------------
------------
>>
>>
------------------------------------------------------------
---------
>> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
>> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
> 
>
------------------------------------------------------------
---------
> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
> 
> 

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

Hi Yana and thanks for your reply,

Yana Stamcheva a écrit :
 > Hey Sympho,
 >
 > When I wrote my mail I didn't see that you've already
committed it 
all. When you are asking for confirmation, which is a very
good idea 
when you're changing someone else's code, you could also
wait for a 
response
 >

Sorry for the haste :(
I will calm down myself and try to avoid rush next time 

 > Could you please have a look at my comments and you
should may be 
turn back to the previous version for some of the changes
you've made. 
(For example the OperationSetMultiUserChatJabberImpl should
definitely 
be turned).

I haven't modified the behaviour here as I saw that opsets
of others 
protocols act in the same way.
only added comments.

see inline for the rest
 >
 > Thanks,
 > Yana
 >
 > Yana Stamcheva wrote:
 >> Hi Sympho,
 >>
 >> see inline.
 >>
 >> Sympho wrote:
 >>> Hi all, Yana,
 >>>
 >>> This is a series of little modifications which
touch several parts 
of code. I want be sure there isn't
 >>> objections before commiting it. I will explain
things in the order 
they appear in the patch file.
 >>>
 >>> first, I added a method
 >>>    public Contact getDefaultContact(Class
operationSet);
 >>> in the metacontact interface and it's
implementation based on
 >>>    public Contact getDefaultContact();
 >>>
 >>> The purpose is to obtain a contact for a
specific operation. For 
example, if
 >>> we right click on a metacontact and we choose
send file, we can simply
 >>> send the file to the contact returned by
 >>>   
metacontact.getDefaultContact(OperationSetFileTransfer.class
);
 >>
 >> Great.
 >>
 >>>
 >>> In ImageLoader.java, I added a static field
 >>> public static final ImageLoader.ImageID
CALL_16x16_ICON
 >>>            = new
ImageID("SIP_COMMUNICATOR_LOGO");
 >>> the image is temporary set to
SIP_COMMUNICATOR_LOGO.
 >>> It is used later for the right button menu
over a metacontact where 
I added a call menu item.
 >>> Yana : I see that there is already some icons
which could fit here. 
Could you
 >>> help me to pick one ?
 >>
 >> May be the most appropriate icon is the one used
for sip online 
status, but don't use it from its current location 
(PROJECT_DIR/resources/images/sip), instead you could copy
it in the 
gui/images/common/ under another more appropriate name. Like
this I 
could change it later.
 >>

done.

 >>>
 >>> ChatSendPanel.java
 >>>   fix a bug discovered long time ago with Emil
and Yana which cause
 >>> exceptions when there isn't sufficient place
in the status label
 >>> (where typing notification are shown) to fit
the status message.
 >>
 >> Ok.
 >>
 >>> ChatContactPanel.java
 >>>    deactivate the action icon on top of the
user name which appears 
on the right
 >>> side of a chat window if action is not
supported.
 >>
 >> Great.
 >>
 >>> ContactRightButtonMenu.java
 >>>    add a call menu item
 >>
 >> Ok.
 >>
 >>>
 >>> ContactList.java
 >>>    popup the right menu at the right
coordinates. Currently, it 
popups only at the good
 >>> height.
 >>
 >> Actually this is not a bug, I thought it's looking
better like this. 
Please wait till I give it one more try before
committing
 >>

Ok, it's just that it surprised me a little, when I clicked
on the 
middle of a meta contact label
with the SC main window maximized. The menu popepup really
far from 
where I expected, and
as mouse menus usually opens where the user click, I haven't
thought it 
was intentionally.

 >>>
 >>> CallManager.java
 >>>    private Contact
getTelephonyContact(MetaContact metaContact)
 >>> now returns 
metaContact.getDefaultContact(OperationSetBasicTelephony.cla
ss);
 >>> rather than the first found telephony
contact.
 >>> Yana : I think if you agree with this point,
the method could be 
suppressed in favor of
 >>> a direct call to 
metaContact.getDefaultContact(OperationSetBasicTelephony.cla
ss);
 >>> wherever it is needed.
 >>
 >> I agree.
 >>
 >>>
 >>> added method
 >>>    public void createCall(Contact contact)
 >>
 >>
 >> Actually there's already a method
createCall(Vector contacts), which 
is meant to be used for selected contacts. The idea is to
have the 
possibility of a multiple selection, which creates a
conference call. I 
think it should be used even if we have only one selected
contact. 
However it could be changed to private and maybe instead of
Vector it's 
better to pass a List.
 >>>
 >>>
 >>> CallPanel.java
 >>>    added a new constructor
 >>>    public CallPanel(CallManager callManager,
Contact contact)
 >>
 >> The same. There's already a constructor, which
takes a list of contacts.
 >>

In fact, I thought of adding that method in CallManager and
modify
CallPanel accordingly only because the underlying telephony
opset 
interface has also two methods
    createCall(String)
    createCall(Contact)
and as calling a single contact is probably the most common
scenario, I 
felt that it was a little
"cumbersome" to create a vector (or a list) each
time we have to do that 
task.

 >>>
 >>> NightlyBuildID.java : replaced "CVS"
by "Sam" as it more kind
 >>>    well, will remove it before committing 
 >>>    This said, it should be SVN, isn't it ?
 >>
 >> I don't see where is the CVS thing
 >>
 >>>
 >>> OperationSetMultiUserChatJabberImpl.java
 >>>        public ChatRoom findRoom(String
roomName)
 >>> returns createLocalChatRoomInstance if the
room searched for
 >>> doesn't exist, but, in
TestOperationSetMultiUserChatJabberImpl,
 >>> findRoom is supposed to return null in case of
an inexisting room. 
I assumed that
 >>> the test expectation was the right behaviour.
Isn't it ?
 >>
 >> Actually it's not so simple. The findRoom is meant
to search for a 
room on the server and not only in the local rooms cache.
You could see that
 >> createLocalChatRoomInstance takes a MultiUserChat
instance, so if 
there's no such room on the server it will throw an 
OperationFailedException, when trying to create the multi
user chat.
 >>

At this point, I reverted modifications which may need more
discussions.
And once again, thanks for your replies.

    Sympho

 >>>
 >>> OperationSetBasicTelephonyJabberImpl.java
 >>>    cleanup, replaced the null stun server with
"stun.iptel.org".
 >>> The stun server provided here is temporary
but, isn't better to have
 >>> a valid server (even if the remote machine is
down) rather than using
 >>> a null one ? And I think anything is better
than null for good.
 >>
 >> I'm not a specialist in this, but it sounds me
right
 >>
 >> Thanks Sympho!
 >>
 >> You have done a really good work!
 >>
 >> Yana
 >>
 >>>
 >>> Well that's all. These modifications aren't
all related, but as 
they aren't substancial,
 >>> I preferred to put them all in a mail. If
someone want to discuss 
only a particular
 >>> point there isn't any obligation to care about
others 
 >>>
 >>> Regards
 >>>    Sympho.
 >>>
 >>>
 >>>
 >>>
 >>>
 >>> 
------------------------------------------------------------
------------
 >>>
 >>>
------------------------------------------------------------
---------
 >>> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
 >>> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
 >>
 >>
------------------------------------------------------------
---------
 >> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
 >> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
 >>
 >>
 >
 >
------------------------------------------------------------
---------
 > To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
 > For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
 >

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

Hi Sympho,

> 
> Ok, it's just that it surprised me a little, when I
clicked on the 
> middle of a meta contact label
> with the SC main window maximized. The menu popepup
really far from 
> where I expected, and
> as mouse menus usually opens where the user click, I
haven't thought it 
> was intentionally.

Actually you're right, it's better as you made it. I just
wanted to have
a look.

> In fact, I thought of adding that method in CallManager
and modify
> CallPanel accordingly only because the underlying
telephony opset 
> interface has also two methods
>    createCall(String)
>    createCall(Contact)
> and as calling a single contact is probably the most
common scenario, I 
> felt that it was a little
> "cumbersome" to create a vector (or a list)
each time we have to do that 
> task.

You're right that the Vector is useless, but we get an array
of selected 
objects and we could pass it directly to the createCall
method. I don't 
see the difference though, in terms of performance, because
we'll always 
have to consider two cases (single and multiple selection)
and this code 
is executed only when you click on the call button. I think
that if we 
add another method this will only complicate the code.

Regards,
Yana




------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net