Looks fine.

On 04/01/16 12:41, Avik Niyogi wrote:
Hi All,
Please review the following incorporated with inputs provided
http://cr.openjdk.java.net/~aniyogi/8041894/webrev.01/

With Regards,
Avik Niyogi
On 28-Dec-2015, at 7:05 pm, Sergey Bylokhov
<sergey.bylok...@oracle.com <mailto:sergey.bylok...@oracle.com>> wrote:

Hi, Avik
On 21/12/15 07:17, Avik Niyogi wrote:
Hi Sergey,
I verified that the issue is only with Aqua Look and feel and not other
look and feels.

It will be better to prove it in the test, so the bug will not occur
again in some other/new L&F.

The type cast for ComponentOrientation was done in the code just for two
reasons:
1) User readability for the component within the event e.
2) The cast for which type it is specified. For example, it can be noted
that in the older code,

This is unrelated to the code style. The cases which you selected are
necessary because we pass these values to the replaceEditor(), note
that parameters of replaceEditor are JComponent, so we must cast in
these cases. Same for (ComponentOrientation) e.getNewValue() because
we pass it to applyComponentOrientation(ComponentOrientation), but in
case of getOldValue it is not necessary because we compare it to null
only.

Also please do not remove the empty line before package com.apple.laf;


if ("editor".equals(propertyName)) {
                    final JComponent oldEditor = *(JComponent)*
e.getOldValue();
                    final JComponent newEditor = *(JComponent)*
e.getNewValue();
                    ui.replaceEditor(oldEditor, newEditor);
                    ui.updateEnabledState();
                } else if ("componentOrientation".equals(propertyName)) {
                    ComponentOrientation o
                            = (ComponentOrientation) e.getNewValue();
                    if (o != (ComponentOrientation) e.getOldValue()) {
                        JComponent editor = spinner.getEditor();
                        if (editor != null) {
                            editor.applyComponentOrientation(o);
                        }
                        spinner.revalidate();
                        spinner.repaint();
                    }
Casting of JComponent is done explicitly for the values there.
Maintaining same standard
and scoping out uncertainty seems correct in the context.

With Regards,
Avik Niyogi

On 19-Dec-2015, at 4:47 am, Sergey Bylokhov
<sergey.bylok...@oracle.com
<mailto:sergey.bylok...@oracle.com><mailto:sergey.bylok...@oracle.com>>
wrote:

Hi, Avik.
Looks fine to me. It seems that the cast here is not necessary?
if (o != (ComponentOrientation) e.getOldValue())

Can you confirm that this bug not affectes our other looks and feels?
It will be good to update the test to prove that.

On 17/12/15 10:21, Avik Niyogi wrote:


Hi All,

Kindly review the fix for JDK9.
*Bug*:https://bugs.openjdk.java.net/browse/JDK-8041894

*Webrev*:http://cr.openjdk.java.net/~aniyogi/8041894/webrev.00/

*Issue*: Test case throws an exception as subcomponents of were not
getting orientation
property assignment.

*Cause*: The case where property name as Component orientation not
present to traverse
the property to subcomponents of spinner for Aqua look and feel.

*Fix*: The else if clause for this property name was added while
checking for for editor to
take charge of applying the component orientation to all subsequent
subcomponents.

With Regards,
Avik Niyogi


--
Best regards, Sergey.



--
Best regards, Sergey.



--
Best regards, Sergey.

Reply via email to