dlr-gtlab / gtlab-core

GTlab Core Framework
https://www.gtlab.de
Other
7 stars 2 forks source link

2nd Attempt: "Allow to detach MDI Tabs to make them their own standalone widget" #1210

Closed mariusalexander closed 4 months ago

mariusalexander commented 4 months ago

Description

Closes #604.

See previous PR #1201 for first attempt and more details.

I just could not live without this feature so I gave it another shot and I think I finally could make it work. It not the nicest piece of code I have ever written but I hope it is sufficient. With the redesign of the MDI launcher (see #1199) this code hopefully does not live long anyways.

The widget can be redocked by minimizing the window (using the window manager). This feels somewhat intuitive, but in the we may want to enhance the UX by allowing drag and drop.

How Has This Been Tested?

Manually. I verified that the buttons of the tab widget are deleted properly, that the MDI Item and widget are deleted properly and could not found any issue.

There was an bug where GTlab would crash if I tried to dock the undocked widget a second time, this was reslved by using Qt::QueuedConnection for the redock slot-function.

Screenshots

Before undocking:

grafik

After undocking:

grafik

Checklist:

mariusalexander commented 4 months ago

@jensschmeink @rainman110 I would like both of your reviews. I know I moved the issue to 2-1-0, but I could not live without the feature once implemented. I hope this PR can still make it into 2-0-7 even tough it is not a fix but a new feature. Since 2-1-0 will probably takes us quite a while to finish, I would like to have access to this system now (and I think everybody else would agree :joy:).

jensschmeink commented 4 months ago

@mariusalexander IMHO its ok like this

mariusalexander commented 4 months ago

@rainman110 May you have a look at the DynamicRange class? Do you like the idea or do you think it is unnecessary? One could also add a step size parameter or support negative ranges (e.g. 10...0), but implementing this is not that easy IMO.

rainman110 commented 4 months ago

💋 This is really cool! I just tested it. Great work. Now I just need to look at the code ;)

rainman110 commented 4 months ago

@mariusalexander How does redocking work? I can't figure it out... Maximizing does not work here.

rainman110 commented 4 months ago

One GUI test is failing that seems to be related directly to the new frame code: https://gtlab.pages.gitlab.dlr.de/-/internal/github-mirrors/gtlab-core-mirror/-/jobs/3846663/artifacts/gui_tests_web/index.html

mariusalexander commented 4 months ago

@mariusalexander How does redocking work? I can't figure it out... Maximizing does not work here.

Minimizing should do the trick

mariusalexander commented 4 months ago

I unfortunately found a bug. Drag and Drop is not yet working anymore with undocked MDI widgets. Please fix this before a merge can be approved.

To clarify, you mean drag and drop actions not draging the window back into the central widget area to make it redock...?

Are you sure it does not work, or can you elaborate? I can open the memento viewer and detach it. Once detached I can drag any object into the window and it will display the memento of that object. Thus I think it works as intended.

rainman110 commented 4 months ago

I meant dragging GTObjects into the window. I tested it with the 3D cad kernel and predesign editor. There it did not work but it worked with the docker window. I am gonna test it again

Update 1: The 2D predesign plot still is able to use drag and drop. The 3D plot however not. I don't know yet, where's the difference and why it is not working in the 3D case

Update 2: Dragging to the 2D predesign plot works only sometimes

Update 3: At the beginning, dragging seems to be working. At some point though, dragging stops working with all undocked MDI widgets. Even the memento viewer.

Update 4: I can reproduce it now:

  1. Open something with the predesign 3D viewer
  2. Open the memento viewer
  3. Undock the memento viewer
  4. Drag and drop something to the memento viewer -> Does not work

It seems, that opening the 3D viewer makes all undocked widgets not accepting drags anymore.

mariusalexander commented 4 months ago

It seems, that opening the 3D viewer makes all undocked widgets not accepting drags anymore.

Woa, that is strange!! What should we do about it?

rainman110 commented 4 months ago

Woa, that is strange!! What should we do about it?

I would say, we need to look at the CADKernel and PreDesign and put this as a known issue into the ChangeLog.

mariusalexander commented 4 months ago

Applied your suggestions. I have not yet looked into the issue with the GUI Tests, maybe it fixed itself (fingers crossed)

mariusalexander commented 4 months ago

One GUI test is failing that seems to be related directly to the new frame code: https://gtlab.pages.gitlab.dlr.de/-/internal/github-mirrors/gtlab-core-mirror/-/jobs/3846663/artifacts/gui_tests_web/index.html

Seems to have fixed itself!

mariusalexander commented 4 months ago

I would say, we need to look at the CADKernel and PreDesign and put this as a known issue into the ChangeLog.

Updated the changelog

rainman110 commented 4 months ago

One GUI test is failing that seems to be related directly to the new frame code: https://gtlab.pages.gitlab.dlr.de/-/internal/github-mirrors/gtlab-core-mirror/-/jobs/3846663/artifacts/gui_tests_web/index.html

Seems to have fixed itself!

No, your link leads to one failed test:

 ERROR10:49:57
Script Error
 LookupError: Object '{type='GtGraphicsView' unnamed='1' visible='0' window='{type=\'GtDockableFrame\' unnamed=\'1\' visible=\'0\'}'}' not ready. (Screenshot in "/home/gitlab-runner/builds/3AQtSC_d/0/gtlab/internal/github-mirrors/gtlab-core-mirror/./gui_tests_web/data/gtlab_interface_gui_tests/tst_mdi/errorImages/error_1.png")
mariusalexander commented 4 months ago

One GUI test is failing that seems to be related directly to the new frame code: https://gtlab.pages.gitlab.dlr.de/-/internal/github-mirrors/gtlab-core-mirror/-/jobs/3846663/artifacts/gui_tests_web/index.html

Seems to have fixed itself!

No, your link leads to one failed test:

 ERROR10:49:57
Script Error
 LookupError: Object '{type='GtGraphicsView' unnamed='1' visible='0' window='{type=\'GtDockableFrame\' unnamed=\'1\' visible=\'0\'}'}' not ready. (Screenshot in "/home/gitlab-runner/builds/3AQtSC_d/0/gtlab/internal/github-mirrors/gtlab-core-mirror/./gui_tests_web/data/gtlab_interface_gui_tests/tst_mdi/errorImages/error_1.png")

This was the link you provided originally. Currently all Checks in GitHub pass