UI: Add scroll to sidebar tabs #105355

Guillermo Venegas · 2023-03-02T00:50:15+01:00

Guillermo Venegas commented

2023-03-02 00:50:15 +01:00

When the height of the editor couldn’t fit the sidebar tabs, they would shrink
to a minimum size that made it impossible to read the tab labels.

This pull request matches the behaviour with the Properties Editor navigation
bar, by introducing the following improvements:

Avoid truncating tab labels.
Allow scrolling when tabs don’t fit.

Behaviour is similar to how scrolling works in the Properties Editor navigation
bar, supporting mouse wheel up/down and MMB, and switching tabs with
Ctrl+Wheel Up/Down.

When the height of the editor couldn’t fit the sidebar tabs, they would shrink to a minimum size that made it impossible to read the tab labels. This pull request matches the behaviour with the Properties Editor navigation bar, by introducing the following improvements: * Avoid truncating tab labels. * Allow scrolling when tabs don’t fit. Behaviour is similar to how scrolling works in the Properties Editor navigation bar, supporting mouse wheel up/down and MMB, and switching tabs with `Ctrl+Wheel Up/Down`.

❤️ 6 🎉 4 🚀 11

Pablo Vazquez added this to the User Interface project 2023-03-02 22:45:16 +01:00

Pablo Vazquez commented

2023-03-02 22:49:09 +01:00

Amazing to see this fix again. It works as one would expect, here's a video showing how it looks.

Thanks for working on it!

Amazing to see this fix again. It works as one would expect, here's a [video showing how it looks](/attachments/4aac58a5-a85b-42c4-a660-c59da60e5d15). Thanks for working on it!

sc_-2022-08-17_14.47.30.mp4

190 KiB

Hans Goudey added the

Module

User Interface

label 2023-03-06 15:44:24 +01:00

Campbell Barton reviewed 2023-03-09 06:03:24 +01:00

source/blender/editors/interface/view2d_ops.cc Outdated

						
				@ -483,2 +510,4 @@

				  }

				  wmWindow *win = CTX_wm_window(C);

				  wmEvent *event = win->event_last_handled;

Campbell Barton commented

2023-03-09 06:03:24 +01:00

Use win->eventstate, win->event_last_handled is intended for event handling logic and not to be used by interface logic. Updated the doc-string to note this 09ba0210d9.

Use `win->eventstate`, `win->event_last_handled` is intended for event handling logic and not to be used by interface logic. Updated the doc-string to note this 09ba0210d9a19a2713fa0f92cf5f54914789c62f.

Guillermo Venegas commented

2023-03-09 18:13:03 +01:00

Solved, the problem I had is that mval is not assigned in win->eventstate for 'Wheel Up/Down' events.

Solved, the problem I had is that `mval` is not assigned in `win->eventstate` for 'Wheel Up/Down' events.

guishe marked this conversation as resolved

Campbell Barton requested review from Campbell Barton 2023-03-09 13:07:39 +01:00

Julian Eisel reviewed 2023-03-09 15:02:29 +01:00

source/blender/editors/interface/view2d_ops.cc Outdated

						
				@ -196,6 +206,21 @@ static int view_pan_exec(bContext *C, wmOperator *op)

				  return OPERATOR_FINISHED;

				}

				inline bool mouse_in_category_tab(const ARegion *region, const wmEvent *event)

Julian Eisel commented

2023-03-09 14:47:06 +01:00

I don't see a point in using inline here, this is not a performance sensitive code path, so I rather don't optimize prematurely and let the compiler figure things out.

I don't see a point in using `inline` here, this is not a performance sensitive code path, so I rather don't optimize prematurely and let the compiler figure things out.

guishe marked this conversation as resolved

Guillermo Venegas force-pushed category-tab-scroll from 19509af027 to 721e1b0084

2023-03-09 18:49:35 +01:00

Compare

Campbell Barton requested changes 2023-03-10 00:27:06 +01:00

source/blender/editors/interface/interface_panel.cc Outdated

						
				@ -2256,3 +2245,3 @@

				    if (LIKELY(category)) {

				      PanelCategoryDyn *pc_dyn = UI_panel_category_find(region, category);

				      if (LIKELY(pc_dyn)) {

				      if (LIKELY(pc_dyn) && (event->modifier & KM_CTRL)) {

Campbell Barton commented

2023-03-10 00:00:37 +01:00

Include a comment for shy checking CTRL is needed, also shouldn't this check event->modifier == KM_CTRL (so other modifier combinations are ignored).

Include a comment for shy checking CTRL is needed, also shouldn't this check `event->modifier == KM_CTRL` (so other modifier combinations are ignored).

guishe marked this conversation as resolved

source/blender/editors/interface/view2d_ops.cc

						
				@ -196,6 +206,21 @@ static int view_pan_exec(bContext *C, wmOperator *op)

				  return OPERATOR_FINISHED;

				}

Campbell Barton commented

2023-03-10 00:26:50 +01:00

Use ED_region_panel_category_gutter_isect_xy or add a new public utility function to area_query.c if existing functions cannot be used - with an explanation for how it differs from existing intersection checking functions.

Use `ED_region_panel_category_gutter_isect_xy` or add a new public utility function to `area_query.c` if existing functions cannot be used - with an explanation for how it differs from existing intersection checking functions.

Guillermo Venegas commented

2023-03-10 01:05:39 +01:00

It works, can I just assume that win->eventstate is never null?

It works, can I just assume that `win->eventstate` is never null?

Campbell Barton commented

2023-03-10 01:53:57 +01:00

Yes, you can assume it's never NULL (the doc-string should be updated to note this).

guishe marked this conversation as resolved

source/blender/makesdna/DNA_screen_types.h Outdated

						
				@ -455,6 +455,9 @@ typedef struct ARegion {

				  rcti drawrct;

				  /** Size. */

				  short winx, winy;

				  /** Scroll value for category tab. */

Campbell Barton commented

2023-03-09 23:56:06 +01:00

Doc-string should state what unit this is in.

Also, is it scaled by DPI - does it properly handle changes (loading other peoples files with Hi-DPI?).

Doc-string should state what unit this is in. Also, is it scaled by DPI - does it properly handle changes (loading other peoples files with Hi-DPI?).

guishe marked this conversation as resolved

Campbell Barton reviewed 2023-03-10 00:31:14 +01:00

source/blender/editors/interface/interface_panel.cc Outdated

						
				@ -1405,14 +1400,13 @@ void UI_panel_category_draw_all(ARegion *region, const char *category_id_active)

				  LISTBASE_FOREACH (PanelCategoryDyn *, pc_dyn, &region->panels_category) {

				    const rcti *rct = &pc_dyn->rect;

				    const bool is_visible = rct->ymin < v2d->mask.ymax && rct->ymax > v2d->mask.ymin;

Campbell Barton commented

2023-03-10 00:31:14 +01:00

Couldn't this be more efficient?

if (rct->ymin < v2d->mask.ymax) {
  /* Scrolled outside the top of the view, check the next tab. */
  continue;
}
if (rct->ymax > v2d->mask.ymin) {
  /* Scrolled past visible bounds, no need to draw other tabs. */
  break;
}

Couldn't this be more efficient? ``` if (rct->ymin < v2d->mask.ymax) { /* Scrolled outside the top of the view, check the next tab. */ continue; } if (rct->ymax > v2d->mask.ymin) { /* Scrolled past visible bounds, no need to draw other tabs. */ break; } ```

Guillermo Venegas commented

2023-03-10 01:14:08 +01:00

if (rct->ymin > v2d->mask.ymax) {
  /* Scrolled outside the top of the view, check the next tab. */
  continue;
}
if (rct->ymax < v2d->mask.ymin) {
  /* Scrolled past visible bounds, no need to draw other tabs. */
  break;
}

Works fine!

``` if (rct->ymin > v2d->mask.ymax) { /* Scrolled outside the top of the view, check the next tab. */ continue; } if (rct->ymax < v2d->mask.ymin) { /* Scrolled past visible bounds, no need to draw other tabs. */ break; } ``` Works fine!

guishe marked this conversation as resolved

Campbell Barton commented

2023-03-10 02:26:46 +01:00

Something to consider is how the scroll position is handled across modes.

Edit-mode for example may show many tabs, the user may scroll to the bottom.
Do we consider it a problem if toggling edit-mode (for e.g.) resets the scroll position?

Something to consider is how the scroll position is handled across modes. Edit-mode for example may show many tabs, the user may scroll to the bottom. Do we consider it a problem if toggling edit-mode (for e.g.) resets the scroll position?

Pablo Vazquez commented

2023-03-10 12:05:04 +01:00

Do we consider it a problem if toggling edit-mode (for e.g.) resets the scroll position?

I think it's acceptable since we do that already in other vertical tabs layout like the Properties Editor's navigation bar, or even the editor itself.

If a solution for the navigation bar's scroll comes up it could be applied here, but that would probably be a separate patch that fixes both.

> Do we consider it a problem if toggling edit-mode (for e.g.) resets the scroll position? I think it's acceptable since we do that already in other vertical tabs layout like the Properties Editor's navigation bar, or even the editor itself. If a solution for the navigation bar's scroll comes up it could be applied here, but that would probably be a separate patch that fixes both.

Guillermo Venegas requested review from Campbell Barton 2023-03-10 22:08:35 +01:00

Campbell Barton approved these changes 2023-03-15 09:57:44 +01:00

Guillermo Venegas force-pushed category-tab-scroll from ae62c60f4d to 9478a4d45d

2023-03-15 15:54:30 +01:00

Compare

Guillermo Venegas force-pushed category-tab-scroll from 9478a4d45d to e94fbaa5cf

2023-03-15 16:19:59 +01:00

Compare

Guillermo Venegas force-pushed category-tab-scroll from e94fbaa5cf to c48c4b090c

2023-03-15 16:36:56 +01:00

Compare

Pablo Vazquez merged commit c0e757a713 into main

2023-03-15 16:45:21 +01:00

Pablo Vazquez referenced this issue from a commit

2023-03-15 16:45:22 +01:00

UI: Add scroll to sidebar tabs

Pablo Vazquez deleted branch category-tab-scroll

2023-03-15 16:45:22 +01:00

Alexander Revkov referenced this issue from a commit

2023-03-15 19:19:50 +01:00

UI: Add scroll to sidebar tabs

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

UI: Add scroll to sidebar tabs #105355