Hole dialog discussion

About the development of the Part Design module/workbench. PLEASE DO NOT POST HELP REQUESTS HERE!
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
User avatar
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Re: Hole dialog discussion

Post by uwestoehr »

wmayer wrote: Thu Dec 17, 2020 4:14 pm Additionally git commit 627fea4e2 makes the spin box for the angle of the drill point wider again because with the current size the value is only partially visible.
Many thanks also to rynn.
There is only a minor issue: the angle is not aligned to the other spin boxes:
Attachments
FreeCAD_DoPmmAxNJs.png
FreeCAD_DoPmmAxNJs.png (13.49 KiB) Viewed 1749 times
wmayer
Founder
Posts: 20309
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Hole dialog discussion

Post by wmayer »

There is only a minor issue: the angle is not aligned to the other spin boxes:
This is caused by the horizontal layout of the radio button and the spin box. Funny is that here the spin box is longer instead of shorter as the spin boxes above.
rynn
Posts: 467
Joined: Tue Jul 31, 2018 7:00 am

Re: Hole dialog discussion

Post by rynn »

This can be fixed too, but need changes to the dialog-structure.
Screenshot_20201218.png
Screenshot_20201218.png (109.15 KiB) Viewed 1728 times
Model actual thread… seems to be unaccessible?!
See PR #4162. (lots of changes because file is autogenerated by designer and structure changed).
wmayer
Founder
Posts: 20309
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Hole dialog discussion

Post by wmayer »

git commit 9be9abe47
Yes, the trick is to move the spin box out of widget_2 that includes the two radio buttons.
wmayer
Founder
Posts: 20309
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Hole dialog discussion

Post by wmayer »

wmayer wrote: Fri Dec 18, 2020 3:30 pm git commit 9be9abe47
Yes, the trick is to move the spin box out of widget_2 that includes the two radio buttons.
EDIT:
rynn wrote: Fri Dec 18, 2020 2:54 pm This can be fixed too, but need changes to the dialog-structure.
Just saw your PR after I checked-in my modifications. I overwrote my changes with yours and pushed them to master.
wmayer
Founder
Posts: 20309
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Hole dialog discussion

Post by wmayer »

git commit 1c897b7ba fixes the TAB order
User avatar
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Re: Hole dialog discussion

Post by uwestoehr »

wmayer wrote: Fri Dec 18, 2020 4:06 pm git commit 1c897b7ba fixes the TAB order
One of these commits introduced a regression:
- select a circle sketch and create a hole
- in the dialog select 'threaded'

result: you can no longer select if the thread is left- or right-handed because the radio group is disabled:
Attachments
FreeCAD_NGquNtBUTu.png
FreeCAD_NGquNtBUTu.png (6.83 KiB) Viewed 1679 times
rynn
Posts: 467
Joined: Tue Jul 31, 2018 7:00 am

Re: Hole dialog discussion

Post by rynn »

uwestoehr wrote: Fri Dec 18, 2020 4:28 pm
wmayer wrote: Fri Dec 18, 2020 4:06 pm git commit 1c897b7ba fixes the TAB order
One of these commits introduced a regression:
- select a circle sketch and create a hole
- in the dialog select 'threaded'

result: you can no longer select if the thread is left- or right-handed because the radio group is disabled:
That problem is real, but as far as I can see, it existed befor these patches.
wmayer
Founder
Posts: 20309
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Hole dialog discussion

Post by wmayer »

uwestoehr wrote: Fri Dec 18, 2020 4:28 pm One of these commits introduced a regression:
- select a circle sketch and create a hole
- in the dialog select 'threaded'

result: you can no longer select if the thread is left- or right-handed because the radio group is disabled:
We only modified the .ui file but not the .cpp file, so we didn't change the program logic. The right/left option will be enabled when changing the threaded option three times. So there is an odd behaviour implemented in the .cpp file.

Maybe using a state machine is an option to fix this:
https://doc.qt.io/qt-5/statemachine-api.html
https://doc.qt.io/qt-5/qstatemachine.html
wmayer
Founder
Posts: 20309
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Hole dialog discussion

Post by wmayer »

uwestoehr wrote: Fri Dec 18, 2020 4:28 pm One of these commits introduced a regression:
There is no regression at all but the new behaviour adds a case that is not handled correctly. In previous versions it was not possible to change the option "Threaded" if the profile is set to None. Now you can change this option even if profile is set to None and there an inconsistent initialization of the hole feature becomes noticeable.

The default value of Threaded is false which means that the ThreadDirection property is read-only. However, in the constructor this isn't set and thus the call of

Code: Select all

ThreadDirection.setReadOnly(false);
in

Code: Select all

void Hole::onChanged(const App::Property *prop)
won't trigger an update of the UI.

git commit 5aac143e6
Post Reply