Looks good to me.
Regards,
Rajeev Chamyal
*From:*Avik Niyogi
*Sent:* 24 March 2016 12:54
*To:* Rajeev Chamyal; Alexander Scherbatiy; Sergey Bylokhov;
swing-dev@openjdk.java.net
*Subject:* Re: <Swing Dev> Review Request of 8137169 : [macosx]
Incorrect minimal heigh of JTabbedPane with more tabs
Hi All,
Please review code changes as per inputs received.
http://cr.openjdk.java.net/~aniyogi/8137169/webrev.03
<http://cr.openjdk.java.net/%7Eaniyogi/8137169/webrev.03>
As SCROLL_TAB_LAYOUT is the default layout for Aqua LAF, with
implementation within the derived AquaTruncatedTabbedPaneLayout,
extending of TabbedPaneScrollLayout is not needed and management of
row and column height is done within it itself. Hence
preferredTabAreaWidth and preferredTabAreaHeight need not manage the
number of columns and rows respectively and will remain 1 as tabs can
be brought forth to the UI by the arrow buttons AQUA provides instead
of placing them in another row or column. This is also the expected
behaviour for AquaLAF as per javadoc.
With Regards,
Avik Niyogi
On 24-Mar-2016, at 12:34 pm, Rajeev Chamyal
<rajeev.cham...@oracle.com <mailto:rajeev.cham...@oracle.com>> wrote:
Hello Avik,
x variable on line 2195 is not used anywhere. Do we need for loop
also?
Regards,
Rajeev Chamyal
*From:*Avik Niyogi
*Sent:*24 March 2016 12:19
*To:*Alexander Scherbatiy
*Cc:*Sergey Bylokhov; Rajeev Chamyal; swing-dev@openjdk.java.net
<mailto:swing-dev@openjdk.java.net>
*Subject:*Re: <Swing Dev> Review Request of 8137169 : [macosx]
Incorrect minimal heigh of JTabbedPane with more tabs
Hi All,
Please review my code changes below as per the inputs received.
http://cr.openjdk.java.net/~aniyogi/8137169/webrev.02/
<http://cr.openjdk.java.net/%7Eaniyogi/8137169/webrev.02/>
As SCROLL_TAB_LAYOUT is the default layout for Aqua LAF, with
implementation within the derived AquaTruncatedTabbedPaneLayout,
extending of TabbedPaneScrollLayout is not needed and management
of row and column height is done within it itself. Hence
preferredTabAreaWidth and preferredTabAreaHeight need not manage
the number of columns and rows respectively and will remain 1 as
tabs can be brought forth to the UI by the arrow buttons AQUA
provides instead of placing them in another row or column. This is
also the expected behaviour for AquaLAF as per javadoc.
With Regards,
Avik Niyogi
On 23-Mar-2016, at 4:14 pm, Alexander Scherbatiy
<alexandr.scherba...@oracle.com
<mailto:alexandr.scherba...@oracle.com>> wrote:
On 23/03/16 14:07, Avik Niyogi wrote:
On 23-Mar-2016, at 3:31 pm, Alexander Scherbatiy
<alexandr.scherba...@oracle.com
<mailto:alexandr.scherba...@oracle.com>> wrote:
On 21/03/16 09:19, Avik Niyogi wrote:
Hi Alexander,
I agree with what you said regarding the look and
feel looking different. But this bug arrises due
to setting of TabbedPaneScrollLayout only. If
Scroll Layout is not meant for Aqua look and feel
should not the setting of this parameter instead
throw a helpful error saying this parameter is not
accepted
According to the JTabbedPane.setTabLayoutPolicy(int
tabLayoutPolicy) javadoc:
"Some look and feels might only support a subset of
the possible layout policies, in which case the value
of this property may be ignored."
Aqua L&F ignores WRAP_TAB_LAYOUT for JTabbedPane
tabs layouting and always use SCROLL_TAB_LAYOUT. No
exception should be thrown in this case.
Actually, it is doing the other way around for Aqua L&F.
It is defaulting WRAP_TAB_LAYOUT and setting
SCROLL_TAB_LAYOUT is still setting a subclass of
TabbedPaneLayout and not TabbedPaneScrollLayout.
According to the JTabbedPane javadoc:
SCROLL_TAB_LAYOUT: Tab layout policy for providing a subset
of available tabs when all the tabs will not fit within a
single run.
WRAP_TAB_LAYOUT The tab layout policy for wrapping tabs in
multiple runs when all tabs will not fit within a single run.
The Aqua L&F uses only AquaTabbedPaneUI and
AquaTabbedPaneContrastUI for tabbed pane UI and they both
returns AquaTruncatingTabbedPaneLayout which has been designed
for to place tabs according to the SCROLL_TAB_LAYOUT.
Thanks,
Alexandr.
Thanks,
Alexandr.
instead of absorbing this parameter and letting it
render itself into a dummy node which does not
proceed further with this parameter? Maybe we need
to discuss what the expected behaviour may be.
Also, thank you for the inputs regarding how to
proceed with removing duplicate code.
With Regards,
Avik Niyogi
On 19-Mar-2016, at 1:52 am, Alexander
Scherbatiy <alexandr.scherba...@oracle.com
<mailto:alexandr.scherba...@oracle.com>> wrote:
I would think about something like:
-------------
public class TabbedPaneLayout implements
LayoutManager {
protected int
basePreferredTabAreaWidth(final int
tabPlacement, final int height) {
// TabbedPaneLayout
preferredTabAreaWidth implementation
}
protected int
truncatingPreferredTabAreaWidth(final int
tabPlacement, final int height) {
if (tabPlacement ==
SwingConstants.LEFT || tabPlacement ==
SwingConstants.RIGHT) {
return basePreferredTabAreaWidth(tabPlacement,
height);
}
return
basePreferredTabAreaWidth(tabPlacement, height);
}
protected int
preferredTabAreaWidth(final int tabPlacement,
final int height) {
return
basePreferredTabAreaWidth(tabPlacement, height);
}
}
class TabbedPaneScrollLayout extends
TabbedPaneLayout {
@Override
protected int
basePreferredTabAreaWidth(int tabPlacement,
int height) {
// TabbedPaneScrollLayout
preferredTabAreaWidth implementation
}
}
protected class
AquaTruncatingTabbedPaneLayout extends
AquaTabbedPaneCopyFromBasicUI.TabbedPaneLayout {
@Override
protected int
preferredTabAreaWidth(final int tabPlacement,
final int height) {
return
truncatingPreferredTabAreaWidth(tabPlacement,
height);
}
}
protected class
AquaTruncatingTabbedScrollPaneLayout extends
AquaTabbedPaneCopyFromBasicUI.TabbedPaneScrollLayout
{
@Override
protected int
preferredTabAreaWidth(final int tabPlacement,
final int height) {
return
truncatingPreferredTabAreaWidth(tabPlacement,
height);
}
}
-------------
I just have one more question. The
TabbedPaneScrollLayout is only created in
AquaTabbedPaneCopyFromBasicUI. AquaLookAndFeel
only use AquaTabbedPaneUI or
AquaTabbedPaneContrastUI which do not return
TabbedPaneScrollLayout.
Are there any real cases when the
TabbedPaneScrollLayout is created?
When you enabled the
AquaTruncatingTabbedScrollPaneLayout in the
AquaTabbedPaneUI the JTabbedPane L&F with
SCROLL_TAB_LAYOUT does not look similar to
Aqua L&F:
http://cr.openjdk.java.net/~alexsch/8137169/aqua-scrolled-tabbed-pane.png
<http://cr.openjdk.java.net/%7Ealexsch/8137169/aqua-scrolled-tabbed-pane.png>
The AquaTruncatingTabbedPaneLayout already
contains arrows for hidden tabs navigation.
May be the fix should update the
AquaTruncatingTabbedPaneLayout only?
Thanks,
Alexandr.
On 18/03/16 14:21, Avik Niyogi wrote:
Hi Alexander,
Thank you for the inputs. I agree with you
and did feel the need for removing
duplicate code as well. But as per an
earlier review input, changes to the super
call lay outing is not accepted. This was
the only other feasible solution. Created
redundant code in this process, but would
be maintaining with requirements with code
impact to superclasses.
Please provide any insight to a probable
compensate to mitigate this dichotomy of
code expectation. Thank you in advance.
With Regards,
Avik Niyogi
On 18-Mar-2016, at 2:42 pm, Alexander
Scherbatiy
<alexandr.scherba...@oracle.com
<mailto:alexandr.scherba...@oracle.com>>
wrote:
It is not usually a good idea to have
a duplicated code which should be
updated every time in several places.
Is it possible to move the methods
used both in
AquaTruncatingTabbedPaneLayout and
AquaTruncatingTabbedScrollPaneLayout
to TabbedPaneLayout with different
names and then reused?
Thanks,
Alexandr.
On 17/03/16 17:17, Avik Niyogi wrote:
Hi Alexander,
The issue only applies for
ScrollingTabbedPane and hence this
fix.
With Regards,
Avik Niyogi
On 16-Mar-2016, at 4:51 pm,
Alexander Scherbatiy
<alexandr.scherba...@oracle.com
<mailto:alexandr.scherba...@oracle.com>>
wrote:
Does the same issue affects
the AquaTabbedPaneContrastUI?
Thanks,
Alexandr.
On 14/03/16 09:04, Avik Niyogi
wrote:
Hi All,
A gentle reminder, please
review code changes.
With Regards,
Avik Niyogi
On 08-Mar-2016, at
9:51 pm, Avik Niyogi
<avik.niy...@oracle.com
<mailto:avik.niy...@oracle.com>>
wrote:
Hi All,
Please review code
changes done as with
inputs provided.
http://cr.openjdk.java.net/~aniyogi/8137169/webrev.01/
<http://cr.openjdk.java.net/%7Eaniyogi/8137169/webrev.01/>
Also, albeit the title
of issue mentioned is
as above, the
injection of issue
occurs because
*pane.setTabLayoutPolicy(JTabbedPane.SCROLL_TAB_LAYOUT);*is
not honoured.
In the new fix as
provided, references
to base class layout
manager is removed in
current solution.
With Regards,
Avik Niyogi
On 02-Mar-2016, at
7:50 pm, Alexander
Potochkin
<alexander.potoch...@oracle.com
<mailto:alexander.potoch...@oracle.com>>
wrote:
Hello Avik
Let me make it
clear I don't
approve the
proposed fix
and ask you to do
additional evaluation.
Every LookAndFeel
is different and
it doesn't make
much sense
to compare Metal
LaF with AquaLaf.
The AquaLaf mimics
the native MacOS
controls and
therefore look
quite different
from any other Lafs.
The bug you are
fixing has the
following subject
"Incorrect minimal
heigh of
JTabbedPane with
more tabs"
Could you please
fix exactly the
problem with the
minimal heights,
without changing
the UI delegate class.
Thanks
alexp
Gentle
reminder.
Please review
this fix.
On
26-Feb-2016,
at 10:39
am, Avik
Niyogi
<avik.niy...@oracle.com
<mailto:avik.niy...@oracle.com>>
wrote:
The issue
is with
setting
of
TabbedPane*Scroll*Layout()
for the
option
JTabbedPane.SCROLL_TAB_LAYOUT
as is
enabled in
the test code
and*not*TabbedPaneLayout()
as which
is the
default.
The
minimum
size fixes
itself
because
the*ScrollLayout*check
fails in
*setTabLayoutPolicy*()
for the
pane. So
the issue
is with
the call
to set
layout
manager.
There are
only two
configurations
that
the*JTabbedPane*can
exist in
of
which*SCROLL_TAB_LAYOUT*is
one of them.
Fixing the
minimum
size
in*AquaTabbedPaneUI*will
fix it
for*TabbedPaneLayout*()
only which
is
the*WRAP_TAB_LAYOUT*.
Also, I
have
checked
other
implementations
such as
for
*Metal*and*Motif*and*they
have
similar
code for
doing this
process*.
Hence,
with
in-depth
analysis,
this fix
has no
other
impact
apart from
this fix.
In case
the impact
caused by
this
change has
caused
some
definitive
regressions,
please
mention
them so
they can
be
addressed.
Thank you.
With Regards,
Avik Niyogi
On
25-Feb-2016,
at
6:45
pm,
Alexander
Potochkin
<alexander.potoch...@oracle.com
<mailto:alexander.potoch...@oracle.com>>
wrote:
Hello Avik
AquaTruncatingTabbedPaneLayout
has a
lot of
code
which
is
specific
for
the
AquaTabbedPaneUI.
I
don't
think
setting the
layout
manager from
the
base
class
is the
right
solution
here.
If
there
is a
problem with
minimum size
it
should
be
fixed
inside
the
AquaTabbedPaneUI
Thanks
alexp
On
2/24/2016
12:07,
Avik
Niyogi
wrote:
Hi
All,
Kindly
review
the bug
fix for
JDK 9.
*Bug:*
https://bugs.openjdk.java.net/browse/JDK-8137169
*Webrev:*
http://cr.openjdk.java.net/~aniyogi/8137169/webrev.00/
<http://cr.openjdk.java.net/%7Eaniyogi/8137169/webrev.00/>
*Issue:*
For Aqua
Look&Feel,
multiple
calls
to
pane.getMinimumSize().height
causes
incremental
return
of
values.
*Cause:*
The impact
was caused
by
a
major
broken
code
within
AquaTabbedPaneUI.java
for
createLayoutManager()
*Fix:*
Major
linking
calls
to
super
class
fix done
within
createLayoutManager().
With
Regards,
Avik
Niyogi
