<Swing Dev> Review Request of 8137169 : [macosx] Incorrect minimal heigh of JTabbedPane with more tabs
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Wed Mar 23 10:01:38 UTC 2016
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.
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.scherbatiy at oracle.com
>> <mailto:alexandr.scherbatiy at 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
>>
>> 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.scherbatiy at oracle.com
>>>> <mailto:alexandr.scherbatiy at 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.scherbatiy at 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.niyogi at oracle.com> wrote:
>>>>>>>>
>>>>>>>> Hi All,
>>>>>>>>
>>>>>>>> Please review code changes done as with inputs provided.
>>>>>>>> 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.potochkin at 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.niyogi at 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.potochkin at 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/
>>>>>>>>>>>>>
>>>>>>>>>>>>> *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
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160323/062fb238/attachment.html>
More information about the swing-dev
mailing list