<Swing Dev> Fwd: Add keyboard accessibility to JColorChooser swatch
Sean Chou
zhouyx at linux.vnet.ibm.com
Mon Aug 27 06:53:21 UTC 2012
Hi Pavel,
Modifications except the auto testcase are done, I'm still working on
the testcase.
Please take a look at http://cr.openjdk.java.net/~zhouyx/OJDK-61/webrev.04/ .
About code convention, I modified spaces beside +/- operations in paint
method(it was not added because the code above doesn't have spaces).
I tried the " g.setXORMode(Color.WHITE); g.setColor(Color.BLACK); " (it is
only commented out in the patch, I'll remove it in later versions with
testcase), the mark is not obvious for light blue-pink area and dark green
area, as this image:
http://cr.openjdk.java.net/~zhouyx/OJDK-61/screenshot_4.png . And it will
also delete the right and bottom line. So, the current color for mark looks
better.
On Wed, Aug 22, 2012 at 9:16 PM, Pavel Porvatov
<pavel.porvatov at oracle.com>wrote:
> **
> Hi Sean,
>
> Hi Pavel,
>
> I updated the patch according to your comments except No.1 and
> No.4, it is now at: http://cr.openjdk.java.net/~zhouyx/OJDK-61/webrev.03/ .
>
> About comment No.1 ( When right-to-left orientation the Recent
> swatches inverts right and left button ), I tried to set the locale to
> ar_AE, but didn't get a visual result about what I should do, please look
> at http://cr.openjdk.java.net/~zhouyx/OJDK-61/screenshot_3.png . Can
> anyone give a little help on how to produce a right-to-left orientation in
> demo SwingSet2 or others?
>
> I attached the test. It will be useful to add some tests to your change
> (e.g. show JColorChooser with right-to-left orientation, pressing some keys
> by awt Robot and checking the result)
>
>
> About comment No.4(Why new listeners are Serializable ), the
> original MouseListeners in this class are Serializable and I see javadoc
> for Component class says "Developers will need, as always, to consider the
> implications of making an object serializable" .
>
> You are absolutely right about serialization. But if you take a look at
> components serialization/deserialization you will see that BEFORE
> serialization javax.swing.plaf.ComponentUI#uninstallUI is invoked and AFTER
> deserialization javax.swing.plaf.ComponentUI#installUI is invoked. So
> serialization is not broken when new listeners added and removed correctly
> with instalUI/uninstallUI methods.
>
>
>
> Please take a look again, thanks.
>
> Some additional comments below:
> 1. You should assign null to mainSwatchKeyListener and
> recentSwatchKeyListener in the uninstallChooserPanel method
> 2. Do we have bug in bugs database for your fix? If no, could you please
> file a bug with correspondent description. Use that bug number as a folder
> name for the webrev, please
> 3. The code can be a little bit optimized. There is no need to call
> repaint every time in code like this:
> + if (selRow > 0) {
> + selRow--;
> + }
> + repaint();
>
> I'd prefer
> + if (selRow > 0) {
> + selRow--;
> + repaint();
> + }
> In case KeyEvent.VK_HOME and KeyEvent.VK_END such optimization doesn't
> look reasonable for me...
> 4. Could you please follow our code guide lines (spaces etc).
>
> http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-141388.html
> 5. I have some concerns about selection mark in paintComponent:
> a. On light swatches (e.g. on white) the mark looks like it shifted on 1
> pixel
> b. The color selection looks tricky. Does the following code from
> DiagramComponent#paintComponent works
> g.setXORMode(Color.WHITE);
> g.setColor(Color.BLACK);
> ?
>
> Regards, Pavel
>
> On Thu, Aug 16, 2012 at 10:00 PM, Pavel Porvatov <
> pavel.porvatov at oracle.com> wrote:
>
>> Hi Sean,
>>
>>
>> Updated the repository used in webrev from jdk8-tl to
>> http://hg.openjdk.java.net/jdk8/awt/jdk .
>>
>> new webrev: http://cr.openjdk.java.net/~zhouyx/OJDK-61/webrev.01/
>>
>> I have the following comments about the fix:
>>
>> 1. When right-to-left orientation the Recent swatches inverts right and
>> left button.
>> 2. Could you please don't use package visibility when
>> fileds/methods/inner classes can be private (e.g. field
>> mainSwatchKeyListener)
>> 3. I think you should uninstall the introduced listeners in the
>> DefaultSwatchChooserPanel#uninstallChooserPanel method
>> 4. Why new listeners are Serializable?
>> 5. I recommend to use if condition instead of switch/case blocks with one
>> branch
>> 6. Could you please rename selrow (and similar variables) into selRow
>> 7. Can we use Component#isFocusOwner instead of supporting new variable
>> showcursor?
>> 8. Could you please follow our code guide lines (spaces etc)
>>
>> Regards, Pavel
>>
>>
>>
>> ---------- Forwarded message ----------
>> From: Sean Chou <zhouyx at linux.vnet.ibm.com>
>> Date: Thu, Aug 9, 2012 at 3:29 PM
>> Subject: <Swing Dev> Add keyboard accessibility to JColorChooser swatch
>> To: swing-dev at openjdk.java.net
>>
>>
>> Hi all,
>>
>> This is a patch to add keyboard accessibility to JColorChooser
>> swatch, so when using TAB, the focus can move to SwatchPanel, choose color
>> with arrow keys and select color with space bar.
>>
>> In current implementation, it is not able to move the focus
>> to SwatchPanel with TAB, this can be seen in SwingSet2 demo.
>> Steps:
>> 1. run SwingSet2 demo
>> 2. click on JColorChooser demo
>> 3. click Background button and Swatches panel appears.
>> 4. Press Tab key => focus moves to OK button as shown in this image
>> http://cr.openjdk.java.net/~zhouyx/OJDK-61/screenshot_1.png
>>
>> With this patch, in step4, focus moves to SwatchPanel, as shown here
>> http://cr.openjdk.java.net/~zhouyx/OJDK-61/screenshot_2.png
>> Then, arrow keys can be used to choose color and select color by space
>> bar.
>>
>> The webrev is http://cr.openjdk.java.net/~zhouyx/OJDK-61/webrev.00/ .
>>
>> Please take a look.
>>
>>
>> --
>> Best Regards,
>> Sean Chou
>>
>>
>>
>>
>> --
>> Best Regards,
>> Sean Chou
>>
>>
>>
>>
>
>
> --
> Best Regards,
> Sean Chou
>
>
>
> import javax.swing.*;
> import java.awt.*;
>
> public class JColorChooserTest {
> public static void main(String[] args) {
> SwingUtilities.invokeLater(new Runnable() {
> @Override
> public void run() {
> JFrame frame = new JFrame();
>
> JColorChooser chooser = new JColorChooser();
>
> chooser.setComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT);
> frame.add(chooser);
>
> frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
> frame.setSize(600, 400);
> frame.setVisible(true);
> }
> });
> }
> }
>
>
--
Best Regards,
Sean Chou
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20120827/285b3287/attachment.html>
More information about the swing-dev
mailing list