Add `vertical_mode` toggle to TabBar #63873

Spartan322 · 2022-08-03T12:55:08Z

The additional theme elements were necessary to keep visual consistency in Godot and provide a more intuitive use case for this implementation.
Because of the appearance, reusing the existing algorithms didn't seem to work out as well as I thought it might, the right and close buttons will align on the right side (as would tend to be the result in horizontal tabs) but will still be separated with h_separation to preserve the functionality in the horizontal TabBar. (otherwise they just end up aligned right next to the text which looks off when the text of each tab is of different lengths)
tab_alignment and vertical_tab_alignment will be hidden from the editor depending on whether the TabBar is in vertical mode or not.

Exposed Additions

TabBar:

enum VAlignmentMode {
    VALIGNMENT_TOP,
    VALIGNMENT_CENTER,
    VALIGNMENT_BOTTOM
}
var vertical_tab_alignment : VAlignmentMode = VALIGNMENT_TOP
var vertical_mode : bool = false

func set_vertical_mode(enable : bool)
func get_vertical_mode() -> bool

func set_vertical_tab_alignment(alignment : VAlignmentMode)
func get_vertical_tab_alignment() -> VAlignmentMode

New Theme Elements

Styleboxes

vertical_tab_selected
vertical_tab_unselected
vertical_tab_disabled

Icons

vertical_increment (scroll_button_down)
vertical_increment_highlight (scroll_button_down_highlight)
vertical_decrement (scroll_button_up)
vertical_decrement_highlight (scroll_button_up_highlight)
vertical_drop_mark (vertical_tabs_drop_mark)

Potential Considerations

Should TabContainer should also receive a vertical_mode in this PR?
Should this PR align the tab button and close button to the far side or should that be controlled by some other method?

Adds`VALignmentMode vertical_tab_alignment` to TabBar Adds vertical_tab_selected, vertical_tab_unselected, vertical_tab_disabled styleboxes to default theme Adds vertical_increment, vertical_increment_hight_light, vertical_decrement, and vertical_decrement_highlight to icons to default theme with corresponding svg files.

RedMser

Great to see this feature being implemented! Tested vertical tabs a bit and found some issues:

Close buttons are not positioned correctly
The drag&drop indicator is a vertical line but should be horizontal

RedMser · 2022-08-03T13:39:25Z

scene/resources/default_theme/default_theme.cpp

+ theme->set_icon("vertical_increment", "TabBar", icons["scroll_button_down"]);
+ theme->set_icon("vertical_increment_highlight", "TabBar", icons["scroll_button_down_hl"]);
+ theme->set_icon("vertical_decrement", "TabBar", icons["scroll_button_up"]);
+ theme->set_icon("vertical_decrement_highlight", "TabBar", icons["scroll_button_up_hl"]);


Similar code has to be put into editor_themes.cpp, else vertical mode can't be used correctly in the editor.

Spartan322 · 2022-08-03T13:56:58Z

Close buttons are not positioned correctly

Yeah, I noticed that, just about to fix it up.

The drag&drop indicator is not vertical

I'll see about fixing that after I ensure the close buttons look correct.

RedMser · 2022-08-03T13:57:31Z

scene/gui/tab_bar.cpp

+ tabs.write[i].size_cache = get_tab_height(i);
+ int tab_width = get_tab_width(i);
+
+ if (max_width > 0 && tab_width > max_width) {


I don't really see the point of setting a max width for vertical tabs. For horizontal tabs, it allows saving space, but this is not the case with vertical tabs.

It should be most effective in containers I would think, otherwise the vertical TabBar will have its minimum size set to the longest tab, it doesn't support a specific overflow behavior, but maximum width allows you to control this otherwise. (and I believe you could feasibly implement overflow behavior with this implementation in a script, but I haven't tested such yet)

I haven't tried it in a container, but at least with anchor/offset positioning, it didn't limit the actual tab width, but only the text width (no matter whether clipping was on or off). But if it works like you say, it's indeed useful to have.

To be fair it might be more suitable to name another property and call it tab_max_content_width since it doesn't limit the right aligned elements, (since they don't follow the same content rules as the regular since it was too ugly to be visually useful imo) but given max_tab_width is useless in vertical mode anyway, I had it perform that task, either renaming the property in the editor in vertical mode or using another property would likely be just as suitable.

RedMser · 2022-08-03T13:59:24Z

scene/gui/tab_bar.cpp

@@ -471,40 +594,123 @@ void TabBar::_notification(int p_what) {
 }
 }

-void TabBar::_draw_tab(Ref<StyleBox> &p_tab_style, Color &p_font_color, int p_index, float p_x) {
+void TabBar::_draw_tab(Ref<StyleBox> &p_tab_style, Color &p_font_color, int p_index, float p_tab_offset) {
 RID ci = get_canvas_item();
 bool rtl = is_layout_rtl();

 Color font_outline_color = get_theme_color(SNAME("font_outline_color"));
 int outline_size = get_theme_constant(SNAME("outline_size"));
 int hseparation = get_theme_constant(SNAME("h_separation"));


There is no corresponding v_separation constant. (Even though hseparation does not work like I intuitively expected it to...)

h_separation separates elements in the tab object, since it had no bearing on the TabBar as a whole I skipped it after realizing what it actually does, since the tabs are still horizontally aligned, the behavior for controlling that separation is actually in the style boxes (the separation for the tabs themselves normally just uses the borders and margins)

Adds `vertical_drop_mark` icon Fixes placement of drop mark in vertical mode Fixes close button positioning

RedMser · 2022-08-03T21:29:33Z

editor/editor_themes.cpp

@@ -1174,8 +1174,13 @@ Ref<Theme> create_editor_theme(const Ref<Theme> p_theme) {
 theme->set_icon("decrement_highlight", "TabBar", theme->get_icon(SNAME("GuiScrollArrowLeftHl"), SNAME("EditorIcons")));
 theme->set_icon("increment_highlight", "TabContainer", theme->get_icon(SNAME("GuiScrollArrowRightHl"), SNAME("EditorIcons")));
 theme->set_icon("decrement_highlight", "TabContainer", theme->get_icon(SNAME("GuiScrollArrowLeftHl"), SNAME("EditorIcons")));


Still missing the set_stylebox calls for vertical_tab_*.

fire-forge · 2022-08-04T00:30:12Z

For consistency, I think it would be better to add HTabBar/VTabBar instead of a vertical mode property to TabBar. Internally it can still use a vertical_mode bool that is set in the constructor (like the other nodes with H/V variations do), but it should be exposed in a way that matches all of the other control node types with horizontal and vertical variations.

This would also remove the need for adding new enums, properties, and theme properties, as the H/V variants could use the same ones but with a different layout.

Spartan322 · 2022-08-04T04:14:49Z

For consistency, I think it would be better to add HTabBar/VTabBar instead of a vertical mode property to TabBar. Internally it can still use a vertical_mode bool that is set in the constructor (like the other nodes with H/V variations do), but it should be exposed in a way that matches all of the other control node types with horizontal and vertical variations.

This was the initial suggested approach godotengine/godot-proposals#5031 (comment) and the reason I deferred to implementing this in this way is mostly because it doesn't fit as much of the commonality of the other H/V nodes since it is considered default to use the horizontal tab layout, course I can change this to add H/VTabBar nodes, either is fine for me.

This would also remove the need for adding new enums, properties, and theme properties, as the H/V variants could use the same ones but with a different layout.

The theme elements I can agree with it would be cleaner, I'm still not sure the enum can be removed since that denotes a functionality by its name which without would contribute to confusion. (unless we renamed AlignmentMode's elements to Being/Start, Center, End but is doing that not a breaking change?) As for properties, outside of the two vertical properties, vertical_mode being in the least unavoidable here, the only strange things that may be found beyond common expectations is h_separation is a little odder in vertical mode as the tab button and close button are right aligned in vertical_mode, and max_tab_width only refers to the left align element sizes in vertical mode. I suppose how jank that might be as an approach would also be removed if made into a VTabBar node approach. If we have VTaBar, I suppose we could refurbish h_separation's documented use better, some properties could be added or to VTabBar that wouldn't mean anything in the horizontal version, tab_max_width would also optimally need to be renamed for VTabBar since it only refers to right aligned content in vertical mode and not the tab as a whole.

KoBeWi · 2022-10-24T14:49:36Z

Yeah I agree with having HTabBar and VTabBar. The TabBar could remain as a base class that allows to switch between layouts (see how BoxContainer handles this).

unless we renamed AlignmentMode's elements to Being/Start, Center, End but is doing that not a breaking change?

Enums are serialized as integers, so it would only affect the code that uses them. I think it's acceptable breakage.

MewPurPur · 2023-07-18T13:57:27Z

The SVGs have editor data from inkscape which is why the PR is 1000+ lines long :D They need to be optimized with svgcleaner

I'd further suggest just copying the existing icons after rebasing, since I optimized them, and rotating them 90 degrees. You can get an optimized result for example by copying a SVG's text, pasting it here: https://www.svgviewer.dev/
and adding transform="rotate(90 8 8)" to the path element (the last two letters should be the center of the viewBox, which is usually 16x16). Then clicking Optimize.

kitbdev · 2023-09-27T05:03:27Z

If VTabBar and HTabBar are made then a corresponding VTabContainer and HTabContainer should probably be made as well to match.
also related: godotengine/godot-proposals#1986

Spartan322 requested review from a team as code owners August 3, 2022 12:55

Spartan322 force-pushed the vertical-TabBar branch from c48e058 to 58d8f48 Compare August 3, 2022 12:58

Spartan322 force-pushed the vertical-TabBar branch from 58d8f48 to 1d04764 Compare August 3, 2022 13:21

YuriSizov added enhancement topic:gui labels Aug 3, 2022

YuriSizov modified the milestones: 4.0, 4.x Aug 3, 2022

RedMser reviewed Aug 3, 2022

View reviewed changes

Spartan322 requested a review from a team as a code owner August 3, 2022 19:34

Fix editor lacking vertical_mode theme support

419861e

Adds `vertical_drop_mark` icon Fixes placement of drop mark in vertical mode Fixes close button positioning

Spartan322 force-pushed the vertical-TabBar branch from 686fa64 to 419861e Compare August 3, 2022 19:40

Update VAlignment enums and documentation

7c17ec5

RedMser reviewed Aug 3, 2022

View reviewed changes

Fix lack of editor theme support for vertical tabs

f2899cb

MewPurPur mentioned this pull request Sep 28, 2023

Option to put TabContainer tabs at bottom #82468

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `vertical_mode` toggle to TabBar #63873

Add `vertical_mode` toggle to TabBar #63873

Spartan322 commented Aug 3, 2022 •

edited

Loading

RedMser left a comment •

edited

Loading

RedMser Aug 3, 2022

Spartan322 commented Aug 3, 2022 •

edited

Loading

RedMser Aug 3, 2022

Spartan322 Aug 3, 2022 •

edited

Loading

RedMser Aug 3, 2022

Spartan322 Aug 3, 2022 •

edited

Loading

RedMser Aug 3, 2022

Spartan322 Aug 3, 2022 •

edited

Loading

RedMser Aug 3, 2022

fire-forge commented Aug 4, 2022 •

edited

Loading

Spartan322 commented Aug 4, 2022 •

edited

Loading

KoBeWi commented Oct 24, 2022

MewPurPur commented Jul 18, 2023 •

edited

Loading

kitbdev commented Sep 27, 2023 •

edited

Loading

Add vertical_mode toggle to TabBar #63873

Are you sure you want to change the base?

Add vertical_mode toggle to TabBar #63873

Conversation

Spartan322 commented Aug 3, 2022 • edited Loading

Exposed Additions

New Theme Elements

Styleboxes

Icons

Potential Considerations

RedMser left a comment • edited Loading

Choose a reason for hiding this comment

RedMser Aug 3, 2022

Choose a reason for hiding this comment

Spartan322 commented Aug 3, 2022 • edited Loading

RedMser Aug 3, 2022

Choose a reason for hiding this comment

Spartan322 Aug 3, 2022 • edited Loading

Choose a reason for hiding this comment

RedMser Aug 3, 2022

Choose a reason for hiding this comment

Spartan322 Aug 3, 2022 • edited Loading

Choose a reason for hiding this comment

RedMser Aug 3, 2022

Choose a reason for hiding this comment

Spartan322 Aug 3, 2022 • edited Loading

Choose a reason for hiding this comment

RedMser Aug 3, 2022

Choose a reason for hiding this comment

fire-forge commented Aug 4, 2022 • edited Loading

Spartan322 commented Aug 4, 2022 • edited Loading

KoBeWi commented Oct 24, 2022

MewPurPur commented Jul 18, 2023 • edited Loading

kitbdev commented Sep 27, 2023 • edited Loading

Add `vertical_mode` toggle to TabBar #63873

Add `vertical_mode` toggle to TabBar #63873

Spartan322 commented Aug 3, 2022 •

edited

Loading

RedMser left a comment •

edited

Loading

Spartan322 commented Aug 3, 2022 •

edited

Loading

Spartan322 Aug 3, 2022 •

edited

Loading

Spartan322 Aug 3, 2022 •

edited

Loading

Spartan322 Aug 3, 2022 •

edited

Loading

fire-forge commented Aug 4, 2022 •

edited

Loading

Spartan322 commented Aug 4, 2022 •

edited

Loading

MewPurPur commented Jul 18, 2023 •

edited

Loading

kitbdev commented Sep 27, 2023 •

edited

Loading