search

FreeCAD / FreeCAD

This is the official source code of FreeCAD, a free and opensource multiplatform 3D parametric modeler.
https://www.freecad.org
Other
17.73k stars 3.78k forks source link

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

Closed kadet1090 closed 2 days ago

kadet1090 commented 2 weeks ago

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.

https://github.com/FreeCAD/FreeCAD/assets/747404/c13018f6-26f7-4f0a-a31e-19ab75a6004d

https://github.com/FreeCAD/FreeCAD/assets/747404/d1171ec4-5533-4d6b-a775-1d2e590370f5

https://github.com/FreeCAD/FreeCAD/assets/747404/e4b52671-c5c4-439e-a6a1-0cbf20d21cff

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

obelisk79 commented 2 weeks ago

Excellent improvement!

prokoudine commented 2 weeks ago

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

PaddleStroke commented 2 weeks ago

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 :

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

Also note that https://github.com/FreeCAD/FreeCAD/pull/13571 is going to mess things a bit here.

kadet1090 commented 1 week ago

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

kadet1090 commented 1 week ago

@PaddleStroke I settled with behavior just like we discussed:

https://github.com/FreeCAD/FreeCAD/assets/747404/c13018f6-26f7-4f0a-a31e-19ab75a6004d

https://github.com/FreeCAD/FreeCAD/assets/747404/d1171ec4-5533-4d6b-a775-1d2e590370f5

https://github.com/FreeCAD/FreeCAD/assets/747404/e4b52671-c5c4-439e-a6a1-0cbf20d21cff

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

PaddleStroke commented 1 week ago

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

furgo16 commented 1 week ago

@kadet1090 nice work! For my use cases, I can confirm that it fixes https://github.com/FreeCAD/FreeCAD/issues/13630 and https://github.com/FreeCAD/FreeCAD/issues/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 commented 1 week ago

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 :sweat_smile: 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 commented 1 week ago

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 commented 1 week ago

It should disappear on charging to one of persistent workbenches

prokoudine commented 1 week ago

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.

https://github.com/FreeCAD/FreeCAD/assets/57467/387dac20-0011-42ca-88db-9a96ee66bf98

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:

https://github.com/FreeCAD/FreeCAD/assets/57467/905ca987-d397-4afd-81e4-7c0f5dbe65c6

kadet1090 commented 1 week ago

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 commented 1 week ago

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

kadet1090 commented 2 days ago

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.