[ Fixed ] Draft snap toolbar: several issues

[Roy_043]

I'd like to revitalize this topic and address the 2nd issue.

I would not recomend to have a document observer for snaps. And I also think that it's good if they are disabled when no active document is found.

What other solutions are available to disable the snaps in a no doc state?

[carlopav]

It's not a big problem IMO, because the snaps are not bound to the document, so it is perfectly consistent if you can toggle them before opening a document from my point of view.
Also snaps toolbar and statusbar are not standard freecad toolbars.
Now the toolbar is created in Snapper.makeSnapToolBar(); if we want it to behave like the others the best we could do is to try to convert the Snap toolbar to a standard toolbar (DraftWorkbench.appendToolbar(QT_TRANSLATE_NOOP("Draft", "Draft snap tools"), self.snaps commands)) and later, perform a customization to set each of it's buttons checkable (each_button.setCheckable(True))...

That would be good cause it also would remove a bit of duplicated code.

Unfortunately there is no way to do that for the statusbar, cause I'm not aware of any FC standard way of creating them, so for it, if we want to make it inActive, we have to create a document observer...

But in the end, unless we want the Snaps to be bound to the document, it's more consistent if you can toggle them also when there is no document opened... (my 2 cents)

[Roy_043]

This is different from what you have said before, which is fine of course. But leaving things as they are now, also leaves us with the second issue, which is obviously a bug:

2.
If there is no active document it is possible to click buttons in the snap toolbar or in the statusbar snap section (flyout). This will only toggle the clicked button, and not the corresponding button in the other GUI element, nor will it change the button's snap value.

My suggestion would be to bring back the V0.18 behavior and do this then:

2.A2:
Of course it is also possible to not use a class from gui_base.py when defining the Snap classes.

[carlopav]

Oops, sorry, consistency it's not something that suits me well
I started a Draft PR to address the issue: https://github.com/FreeCAD/FreeCAD/pull/4915
As you suggest, I now really think is better, if the Snap is not bound to the document, to not derive its commands from GuiCommandSimplest, cause it defines a document related command. I added a new base class for snap commands, and derived them from it.
Now, also when no document is active, the 2 toolbars are updated accordingly.

I know i still am missing some bugs that you reported... can you help me to point out the other problems?

Edit: Looking at how it behaves I still think lot can be improved:
at the moment : pressToolbarButton -> runCommand -> (command toggles snap, gets snap status, updates toolbar and statusbar)
probably better if : pressToolbarButton -> runCommand -> command toggles snap -> Snapper updates toolbar and statusbar
what do you think?

[Roy_043]

Update of issues (the first two have already been mentioned):

1. Wrong highlighting when starting without user.cfg.
2. If there is no active document it is possible to click buttons in the snap toolbar or in the statusbar snap section (flyout). This will only toggle the clicked button, and not the corresponding button in the other GUI element, nor will it change the button's snap value.
3. The '_tip' texts in gui_snaps.py are not displayed in the GUI.
4. Adding a Snap button to a custom toolbar is possible but this button will not be toggled.

Issues 3 (ToolTip issue) and 4 remain, but are not that important.

better if : pressToolbarButton -> runCommand -> command toggles snap -> Snapper updates toolbar and statusbar
what do you think?

It is hard for me to judge that but it looks cleaner.

[carlopav]

The '_tip' texts in gui_snaps.py are not displayed in the GUI.

Yes we could also remove them because the tooltip are set in the Snapper itself. Anyway I wonder if we want the Toolbar to behave like the others and be set in initGui.py module, if a little less customization is desirable, and it's enough to just use the isCheckable state of the button.

Adding a Snap button to a custom toolbar is possible but this button will not be toggled.

hmmm, yes, I think a deeper change is needed for this. Perhaps it could also be desirable...
What I imagine is that we could have 2 different ways to add commands to FC:

Code: Select all

Gui.addCommand('Draft_Command', Draft_Command())
Gui.addStatusCommand('Draft_StatusCommand', Draft_StatusCommand())

and setup it so it will produce a checkable button in the toolbar...
but I think this is over my possibilities...
EDIT: what is deleted above is indeed possible.

[Roy_043]

I started a Draft PR to address the issue: https://github.com/FreeCAD/FreeCAD/pull/4915

This has been merged. Thanks. I am marking this topic as fixed.