[RFC][icedtea-web]: Adding control panel to icedtea-web

Wed Dec 1 12:01:49 PST 2010

On 12/01/2010 02:35 PM, Andrew Su wrote:

> I don't see where a border of 1 is being used. I do know that I do
> use three different border values. This is to offset the a little
> misalignment with adding a border to a panel relative to the list on
> the left. I have changed it to use 5 as consistancy with all other
> panels.
>

Hm..personally I would try to avoid relying too much on pixel-perfect 
positioning. But it's really up to you. If you want to keep the borders, 
then so be it.

> Without setting the dimensions, the panel that has the widest width
> will be used as the width. Because of this there will be no word
> wrap, which takes advantage of the<html>  tag to do word wrapping in
> labels. If anything it could be resizeable so that in the case it
> does get covered the user can always re-adjust.
>

That sounds like a good idea.

> The main methods were used to test each component separately. But
> I've now removed all the main methods except for ControlPanel.java
>

Thanks.

>>> + at SuppressWarnings({ "unused", "serial" })
>>
>> Why suppress unused warnings for the entire class?
> Removed.
>>

Sounds good.

>> I dont understand what this does. Middle-click pasting to
>> checkboxes?
> This is for when pasting with middle click. So let's say I had a
> proxy ip that I want to copy and paste, middle clicking in gnome will
> paste it, but the keypress event will not be triggered. I'm a little
> worried about now what if they changed that to say some other
> button... right click?

Ah, are you talking about pasting in text boxes? (I got really confused 
because the comment says pasting to checkboxes). If middle-click pasting 
does not trigger mouse or key events, then that's a bug on the JDK side. 
You dont have to change anything right now though.

>> Please dont leave TODOs like this unless they refer to future
>> work.
> Well it was a TODO. Finished that off now too.

Ah, good :)

>> If I understand this right, it changes the text displayed in one
>> area when a selection changes. Are you sure this is the best way to do
>> this?
> I can not say for certain this way is the best way to do it, however
> it makes it fairly convenient to view the items instead of just
> hovering.
>

I still think you should add descriptions next to the options 
themselves. Oh well, this is a alpha after all. Ok for now.

>> Maybe something like a tooltip to explain an option might be
>> better?
> I was considering adding tooltips for everything later, possibly
> future enhancements.
>

I am still not convinced that having that cardlayout is a good thing. 
But we can leave the actual UI cleanup to usability experts (any 
volunteers?).

>>
>> Swing threading violation again.
>
> All main methods other than the one in ControlPanel.java is now
> removed as I stated above.
>

That's good.

> Once again another patch update, with the above changes.
>
> Thanks for the review!

Thank YOU for the work and the prompt fixes! I am fine with the current 
patch going in.

Cheers,
Omair