A gentle reminder, Please review the code changes as presented below.
With Regards,
Avik Niyogi
> On 24-Mar-2016, at 3:01 pm, Rajeev Chamyal <rajeev.cham...@oracle.com> wrote:
>
> 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/~aniyogi/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/~aniyogi/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/~aniyogi/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 TabbedPaneScrollLayout() for the option
> JTabbedPane.SCROLL_TAB_LAYOUT as is enabled in the test code
> and notTabbedPaneLayout() 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 AquaTabbedPaneUIwill 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
> <https://bugs.openjdk.java.net/browse/JDK-8137169>
>
> Webrev:
>
> http://cr.openjdk.java.net/~aniyogi/8137169/webrev.00/
> <http://cr.openjdk.java.net/~aniyogi/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
>
