Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gui: Move more button to the end WB TabBar #13721

Merged
merged 7 commits into from May 13, 2024

Conversation

kadet1090
Copy link
Contributor

@kadet1090 kadet1090 commented Apr 29, 2024

Please don't squash - I keep the commit history meaningful :)

This changes back placement of the "more" button of the WB TabBar to be at the end, where it should be naturally placed. In order to ensure that it is always visible the control was reworked to show this button always after the tab bar widget which now is dynamically sized. This is behavior that is well known from browsers.

image

This commit also ensures that active workbench is always visible in the TabBar by adding additional temporary tab when necessary. This tab will automatically dissapear when not needed.

Peek.2024-05-05.16-34.mp4
Peek.2024-05-05.16-33.mp4
Peek.2024-05-05.16-31.mp4

Fixes: #13720
Fixes: #13630
Fixes: #13729
Fixes: #13286
Fixes: #13634

@github-actions github-actions bot added the Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD label Apr 29, 2024
@obelisk79
Copy link

Excellent improvement!

@prokoudine
Copy link

Sweet! Do left/right arrow buttons for scrolling the tabbar work as well?

@PaddleStroke
Copy link
Contributor

As said before I'm not a huge fan of the dynamic size. The reason is that toolbars get messed up pretty easily, so adding a dynamic toolbar in the mix sounds like a risky move.
Having said that, the way you implemented it, where there is only one additional tab max, sounds reasonable.

Some questions :

  • What happens if you trigger the 'Preference' action in the menu? Is the tab behavior correctly defined?
  • Does it work properly when the tab bar is set to the corner widget of the menu bar?

Also when used in the right cornerwidget of the menubar, as the tabbar is then right aligned, the additional tab means that all the workbench tabs would move back and forth which is bad. A solution would be that the tab bar have the capacity to put the + button and the additional tab either on the left or the right. And then it should detect if the option 'right corner' is used, and if so put the + to the left.

@kadet1090
Copy link
Contributor Author

kadet1090 commented Apr 30, 2024

As said before I'm not a huge fan of the dynamic size. The reason is that toolbars get messed up pretty easily, so adding a dynamic toolbar in the mix sounds like a risky move.

yes, this can cause some issues if there is another toolbar after it as this would result in the constant movement of the thing on the right. Probably best way would be to ensure that this toolbar occupies as much space as it can so stuff after the tabbar would be pinned to right edge of screen which seems reasonable. This is also pretty common implementation.

This also is true only if we allow the one dynamic tab, without it will still have constant size but the placement of the "+" button will be more natural.

  • What happens if you trigger the 'Preference' action in the menu? Is the tab behavior correctly defined?

It just opens the preferences so works properly.

  • Does it work properly when the tab bar is set to the corner widget of the menu bar?

There you have me - I have not tested that case, for now it segfaults due to how I restructured the code. I'll fix that later today and try it. but it should not be a problem as the size of the wrapping QWidget should be the same.

Also when used in the right cornerwidget of the menubar, as the tabbar is then right aligned, the additional tab means that all the workbench tabs would move back and forth which is bad. A solution would be that the tab bar have the capacity to put the + button and the additional tab either on the left or the right. And then it should detect if the option 'right corner' is used, and if so put the + to the left.

Yeah, this is a problem - but IMHO the problem that we don't see current workbench name if selected from more menu is much bigger of an issue as the state of the system is not presented which is one of most important heuristics of UX. As we discussed earlier - placing the widget in that place is problematic for multiple reasons. IMHO the best solution would be for it to occupy as much space as possible, i.e. be placed right after the last menu entry. This however will not solve moving UI issue as the menu is also dynamic in size.

But as I think of it, the Right Corner is de facto RTL layout - so placing the "+" button on the left would be on the end which is the whole point of this PR. @PaddleStroke what do you think?

@FreeCAD/design-working-group any input would also be appreciated.

@PaddleStroke
Copy link
Contributor

But as I think of it, the Right Corner is de facto RTL layout - so placing the "+" button on the left would be on the end which is the whole point of this PR. @PaddleStroke what do you think?

I'm not sure to understand what you meant here. It seems that you are agreeing with my proposed solution in previous post, ie placing the + button left when right corner. But as you're asking me what I think of it, I think I might be missing something.

@kadet1090
Copy link
Contributor Author

kadet1090 commented Apr 30, 2024

Yeah, when re-reading your comment - I think that we agree.

I'll implement it later today. It would be nice if anyone would be able to test the changes as I did rework a lot here

@PaddleStroke
Copy link
Contributor

Also note that #13571 is going to mess things a bit here.

@kadet1090 kadet1090 marked this pull request as draft May 4, 2024 22:25
@kadet1090 kadet1090 force-pushed the more-at-the-end branch 4 times, most recently from e7c59ca to 7cc1c39 Compare May 5, 2024 14:13
@kadet1090 kadet1090 marked this pull request as ready for review May 5, 2024 14:14
@kadet1090
Copy link
Contributor Author

@FEA-eng @prokoudine @furgo16 this should fix issues created by you. Can you perhaps test it?

@kadet1090
Copy link
Contributor Author

kadet1090 commented May 5, 2024

@PaddleStroke I settled with behavior just like we discussed:

Peek.2024-05-05.16-34.mp4
Peek.2024-05-05.16-33.mp4
Peek.2024-05-05.16-31.mp4

I'll later add guard to set maximum width of this widget so menu will always be displayed.

@PaddleStroke
Copy link
Contributor

Ah yes the left side of the screen is also right aligned. Good catch.

@furgo16
Copy link
Contributor

furgo16 commented May 6, 2024

@kadet1090 nice work! For my use cases, I can confirm that it fixes #13630 and #13634 and. That last bug is not on the PR description, so you might want to add a Fixes: statement for it too.

I've noticed some unexpected behavior, though: if you've got the "Icon only" setting enabled for the TabBar, whenever you open a new tab with +, that setting is ignored and the new tab is loaded with icon and text. Notice the astersk next to the name too.

Captura de pantalla de 2024-05-06 13-19-45

@kadet1090
Copy link
Contributor Author

kadet1090 commented May 6, 2024

Asterisk is intentional as it denotes that this tab is temporary - I'm happy to discuss this, but yeah that does not work well with icon only setting 😅 I'll fix it in a few hours, thanks!

That last bug is not on the PR description, so you might want to add a Fixes: statement for it too.

Added in

@furgo16
Copy link
Contributor

furgo16 commented May 6, 2024

A quick question from reading the PR description:

This commit also ensures that active workbench is always visible in the TabBar by adding additional temporary tab when necessary. This tab will automatically dissapear when not needed.

What situation would make the temporary tab disappear?

@kadet1090
Copy link
Contributor Author

It should disappear on charging to one of persistent workbenches

@prokoudine
Copy link

Built the branch. I can sort of see the scroll buttons now when the TabBar is vertical, but much like with horizontal orientation, they kind of stay semi-hidden.

vokoscreenNG-2024-05-06_23-10-21.mp4

Another issue is that while re-docking from vertical to horizontal is now possible, any time you add a workbench, the entire TabBar goes back to vertical mode:

vokoscreenNG-2024-05-06_23-14-31.mp4

@kadet1090
Copy link
Contributor Author

First issue is in OpenTheme - I'll make PR there to fix styling issues (like too wide toolbar on side) once this is merged.

The second one seems to be that FreeCAD remembers toolbar positions per workbench and so if you load another workbench it goes back on the left. I also find this frustrating, but this is not job for this release cycle.

@kadet1090 kadet1090 force-pushed the more-at-the-end branch 3 times, most recently from 3323f61 to 758b6dd Compare May 6, 2024 21:45
@kadet1090
Copy link
Contributor Author

I rebased the changes onto latest main branch, including ability to drag this toolbar into menu bar and status bar. This includes a bit of required refactor for the new toolbar areas

This refactors implementation of toolbars in menu / status bar a bit. It
introduces enum with all possible areas like it is in Qt that can be
later used to decide what to do based on toolbar placement.
This changes back placement of the "more" button of the WB TabBar to be
at the end, where it should be naturally placed. In order to ensure that
it is always visible the control was reworked to show this button always
after the tab bar widget which now is dynamically sized. This is
behavior that is well known from browsers.

This commit also ensures that active workbench is always visible in the
TabBar by adding additional temporary tab when necessary. This tab will
automatically dissapear when not needed.

Fixes: FreeCAD#13720
Fixes: FreeCAD#13630
This should fix issues when toolbar containing Workbench TabBar suddenly
(or not) changes orientation. It also fixes size policies so toolbar
resizes properly and does not cause window to grow.

Fixes: FreeCAD#13286
Right corner is placed to the right edge of screen, so its natural
growth occours on the left side. Basically it is Right to Left order and
so in that case the "end" is actually on left and so TabBar should grow
in that direction.

Unfortunately it is not possible to simply use RTL Qt feature to handle
that case as it would result in reverse order of workbenches (people will still
read it in LTR order) and icons on the right which is not wanted. That's
custom support is introduced.
This replaces old mechanism that was based on storing tab bar
orientation in user settings with one that delays initialization by half
of a second to ensure that toolbar is placed where in right place.
This fixes segfault that can occour due to keeping reference to QAction
that is supposed to change workbench.
@chennes chennes merged commit e3418ed into FreeCAD:main May 13, 2024
9 checks passed
@kadet1090
Copy link
Contributor Author

Built the branch. I can sort of see the scroll buttons now when the TabBar is vertical, but much like with horizontal orientation, they kind of stay semi-hidden.
vokoscreenNG-2024-05-06_23-10-21.mp4

Another issue is that while re-docking from vertical to horizontal is now possible, any time you add a workbench, the entire TabBar goes back to vertical mode:
vokoscreenNG-2024-05-06_23-14-31.mp4

https://github.com/obelisk79/OpenTheme/pull/58/files

This should fix theme issues that you have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment