Fix size of tab items when large text padding is used #989
Conversation
| @@ -2972,19 +2972,17 @@ boolean setItemSize(GC gc) { | |||
| for (int i = 0; i < items.length; i++) { | |||
| CTabItem tab = items[i]; | |||
| int width = widths[i]; | |||
| if (tab.height != tabHeight || tab.width != width) { | |||
There was a problem hiding this comment.
This was a "dangerous" optimization as it assumes that tab items only have changed (and need some recalculation) if their size changed. That assumptions breaks with this PR.
| return 0; | ||
| } | ||
|
|
||
| private boolean applyLargeTextPadding(CTabFolder tabFolder) { | ||
| private boolean shouldApplyLargeTextPadding(CTabFolder tabFolder) { |
There was a problem hiding this comment.
The present condition results in a large text padding only when showing images is disabled for all tabs (selected and unselected) of the tab folder.
But this leads to no text padding/margin for unselected tabs when showing images is disabled for all 'unselected' tabs.
- Separation between unselected tabs takes a hit,
Similarly, there is no margin or padding applied to the selected tab when only the selected tab image is set to be hidden.
| private boolean shouldApplyLargeTextPadding(CTabFolder tabFolder) { | |
| private boolean shouldApplyLargeTextPadding(CTabItem item) { | |
| CTabFolder folder = item.getParent(); | |
| return (!folder.showSelectedImage && (item.state & SWT.SELECTED) != 0) || | |
| (!folder.showUnselectedImage && (item.state & SWT.SELECTED) == 0); | |
| } | |
There was a problem hiding this comment.
That's true, but that is the behavior even without the changes in this PR. And in particular, that is the behavior that has been existing for years and must not be changed.
The padding-based separation of tabs, as introduced with #785, is only active if images are completely disabled for tabs. This is currently identified by both showing images of selected and unselected tabs is set to false, as the former was always true before #785, so that this way of configuration cannot produce regressions. Still, we should think of more explicit approach to apply text-based padding to a CTabFolder (see #785 (comment)).
Note that #945 is also still open for this.
There was a problem hiding this comment.
In the context of avoiding regression it makes sense to not apply this suggestion. Will evaluate this on #945 👍
...eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolderRenderer.java
Show resolved
Hide resolved
...eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolderRenderer.java
Show resolved
Hide resolved
The recently introduced option to not show images in tab folders currently works as follows: A large padding is applied to the tab item as a separator with the text being centered within this padding. For tabs showing a close icon (which in particular is the currently selected on), this icon is added right at the right end of the tab item, such that the text is centered in between the left end of the tab item and the close button. This does not look perfect and while the padding is necessary as a separator for tabs only containing a text, having close icons and, in case of the selected tab, also a different background is sufficient as a separator. This change adapts the appearance of tab items having a close icon when having images disabled. It uses the same size for the tab as if no close icon was drawn and then reduces the area in which the text is centered to the remaining space left to the close icon. This looks cleaner and has the effect that tabs within a tab folder have a fixed with, such that when changing the selection all tabs keep their size and positions. Only the texts within the tab items change their positions depending on whether the close icon is shown or not.
HeikoKlare
force-pushed
the
iconless-tabs-selected-appearance
branch
from
0bf36e3
to
3779c51
Compare
|
Thanks for your reviews, @shubhamWaghmare-sap @sratz! |
This is follow-up to #785, which introduced an iconless tab folder design that allows to disable tab item images and uses centered text a large padding within tab items as a separator instead. This visualization option for tab folders currently works as follows: A large padding is applied to the tab item as a separator with the text being centered within this padding. For tabs showing a close icon (which in particular is the currently selected on), this icon is added right at the right end of the tab item, such that the text is centered in between the left end of the tab item and the close button.
As discussed downstream in eclipse-platform/eclipse.platform.ui#1071, this does not look perfect and while the padding is necessary as a separator for tabs only containing a text, having close icons and, in case of the selected tab, also a different background is sufficient as a separator. @thomasritter, @sratz and I evaluated different options and came up with the concept for what is proposed in this PR.
This change adapts the appearance of tab items having a close icon when having images disabled. It uses the same size for the tab as if no close icon was drawn and then reduces the area in which the text is centered to the remaining space left to the close icon. This looks cleaner and has the effect that tabs within a tab folder have a fixed width, such that when changing the selection all tabs keep their size and positions. Only the texts within the tab items change their positions depending on whether the close icon is shown or not.
This is a standalone screenshot of how the padding will look like:
This is how it looks now when switching from "Problems" to "Console" tab:
This is how it will look after this PR when switching from "Problems" to "Console" tab:
Note that this also improves the appearance when a tab folder only has a single item, as it will not have a large padding at the left.
Here is how it looks before this PR:
Here is how it looks after this PR:
@shubhamWaghmare-sap @praveen-skp FYI as original contributors of this functionality.