Hi Sergey and Alexander, Please review the code changes done.
With Regards, Avik Niyogi > On 28-Mar-2016, at 1:45 pm, Avik Niyogi <avik.niy...@oracle.com> wrote: > > 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 >> <mailto: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 <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 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
