Hi Alexander,
With inputs (code) provided, it is not possible to accommodate the right
methods for tabPlacement parameter.
Also, since AquaTabbedPane is copied from BasicUI.java, this implementation if
moved to TabbedPaneLayout, does not accommodate Awua related changes and hence,
the redundant code is in effect.
Also, regarding the query about the below:
> The AquaTruncatingTabbedPaneLayout already contains arrows for hidden tabs
> navigation.
> May be the fix should update the AquaTruncatingTabbedPaneLayout only?
The defect due to the bug is injected only if Scrolling is in effect. The
non-scrolling values populated for tabbedPaneLayout tried to add another row
and hence would result in defects.
This was due to the fact that TabbedPaneScrollLayout was not even getting set
even when the user was trying to set it.
Ideally, it would have been easier to call the super class to manage just the
layout part but as per inputs received, it will not be accepted even though it
have no splash impact and would not lead to regressions.
Due to those reasons, this is the solution seeming fit for this.
I have tried the way suggested in your input code, but it broke the UI and
different elements not required to be visible were being rendered because it is
copying those elements from BasicUI.
If a better solution is available without breaking some code apart from fix
provided, please let me know. Thank you in advance.
With Regards,
Avik Niyogi
> On 19-Mar-2016, at 1:52 am, Alexander Scherbatiy
> <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/~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.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 <
>>>>> <mailto:alexandr.scherba...@oracle.com>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 <
>>>>>>> <mailto:avik.niy...@oracle.com>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/%7Eaniyogi/8137169/webrev.01/>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 <
>>>>>>>> <mailto:alexander.potoch...@oracle.com>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 <
>>>>>>>>>> <mailto:avik.niy...@oracle.com>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 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 <
>>>>>>>>>>> <mailto:alexander.potoch...@oracle.com>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
>>>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8137169>
>>>>>>>>>>>>
>>>>>>>>>>>> Webrev:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8137169/webrev.00/>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
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
