Ticket #4079 - Click inside property value, click Discard, reference to spreadsheet-based reference is erased

[bgoodr]

This seems likely to be a bug, but I do not know if it is a regression from prior builds, since the issue at https://forum.freecadweb.org/viewtopic. ... 6&start=10 (using spaces in Constraint Names) may have obscured my even seeing this problem:

Using freecad-daily build of:

Code: Select all

OS: Ubuntu 19.04 (ubuntu:GNOME/ubuntu)
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.19.
Build type: Release
Python version: 3.7.3
Qt version: 5.12.2
Coin version: 4.0.0a
OCC version: 7.3.0
Locale: English/United States (en_US)


$ dpkg --list | grep -i freecad 
ii  freecad-daily                              0.19~pre1~201908021925~ubuntu19.04.1               all          Extensible Open Source CAx program
ii  freecad-daily-common                       0.19~pre1~201908021925~ubuntu19.04.1               all          Extensible Open Source CAx program - common files
ii  freecad-daily-python3                      0.19~pre1~201908021925~ubuntu19.04.1               amd64        Extensible Open Source CAx program - Python 3 binaries
ii  freecad-daily-runtime                      0.19~pre1~201908021925~ubuntu19.04.1               all          Extensible Open Source CAx program - runtime files
ii  libfreecad-daily-python3-0.19              0.19~pre1~201908021925~ubuntu19.04.1               amd64        Extensible Open Source CAx program - Python 3 library files
$ 

Find attached a zip file with two small files, one being a file containing a spreadsheet, and another containing a single rectangle padded to a length, where the link is referencing an alias in a spreadsheet in another file (all within the same directory that will show up when you extract the zip file.


Steps to reproduce, showing how I created the two documents from scratch:

1. Open up FreeCAD.
2. Switch to Spreadsheet Workbench.
3. Create a new Spreadsheet. Save it as "MasterSpreadsheet".
4. Inside the spreadsheet add into a cell the value "42mm".
5. Right mouse click upon that cell, select Properties..., then select Alias, and add an alias called "flubber".
6. Save that file.
7. Create a new document, and save that as "TopDesign".
8. Switch to Part Design.
9. Create a Body.
10. Create a sketch on that body.
11. Add a rectangle and exit out of the sketch.
12. Pad the sketch, and for the length use this expression:

    Code: Select all

    MasterSpreadsheet#Spreadsheet.flubber

The above is for creating the two document files from scratch. If you are starting the from attached zip file, then do open up both the MasterSpreadsheet and TopDesign files for the following:

1. Select the TopDesign, and select the Model view.
2. In the Model view, see the value there shows the above expression to the right of the value (for me I am using imperial units):
   
3. Click inside the field and wait a bit, and see the tooltip:
   
4. Click on the "fx" button on the far right side of the field, and see:
   
5. Just click inside the value of the field, and don't change anything at all, and then click the Discard button.
6. Look in the Model View, see:
   
7. Now repeating the above at this point never shows the formula again. It is as if now the value is hardcoded to whatever was previously calculated, but the reference to the formula using the value in the spreadsheet is now gone.
8. However, if you repeat the above (you must exit completely out and reload the two docs) but instead of clicking on the Discard button, you instead type the Escape key to exit out of the field, the value is not lost.

I suspect there is something different for how Discard is working than how the Escape key is handled. What I expect is this: if I do not change the value at all, or even if I do change it but then click Discard, it should work as the Escape key works now.
Can this be user-error?

Thanks,
bg

[chrisb]

"Discard" does not mean "Cancel". It discards the expression and makes the property value a normal value again.

Thank you for a great bug report. The details made it easy and quick to answer.

[kisolre]

A "cancel" button is probably not needed and so not programmed there because closing the dialog with its "X" acts one and so no need to extra place to click. Maybe a tooltip for the Discard button explaining briefly what it does will be Ok?

[chrisb]

A "cancel" button is probably not needed and so not programmed there because closing the dialog with its "X" acts one and so no need to extra place to click. Maybe a tooltip for the Discard button explaining briefly what it does will be Ok?

I am not a native English speaker, but never had any doubts about what discard means here.

[bgoodr]

A "cancel" button is probably not needed and so not programmed there because closing the dialog with its "X" acts one and so no need to extra place to click. Maybe a tooltip for the Discard button explaining briefly what it does will be Ok?

I am not a native English speaker, but never had any doubts about what discard means here.

I reason thusly (do note that I'm not criticizing the use of different human languages or the developers that use those human languages; just theorizing as to the situation):

I theorize that it might have been the case that developer, who chose "Discard" for that dialog box, is not a native English speaker. Or, the text strings were translated from some original non-english meaning, into the word "Discard". So, if that was the case, then "Discard" would seem to be a reasonable choice of the button to their eyes. I don't know that for sure; it is pure speculation on my part here. As a native English speaker, who actually hates this language he is stuck with, that is eminently forgivable!!

Assuming the above is correct, then that leads me to the following:

As a native English speaker, instead of "Discard", I would have chosen "Reset" to indicate that values will be changed by the action of clicking that button. "Reset"-ing the values is a mutative operation (changes something held precious by the user), while hitting the Escape key should not change any value.

Having an additional "Cancel" button might be less important to the power-users, as the "dialog with its 'X"" handles that operation. But, for new users coming from other applications, then the "Cancel" button (even if it is redundant with the "X" button), might be good to have, as long as there is sufficient real-estate to house it (might not be, on low-resolution screens). The primary issue here is the meaning of "Discard".

The issue I take with this is that "Discard" can be interpreted, by (this?) English speaker, as "Discard my edits and leave the value unchanged" which is synonymous with what I believe to be the interpretation of a "Cancel" operation (or that "X" button on the dialog itself), and also synonymous with what the Escape key behavior is doing today.

As justification for the above conclusions, I refer to the following quote from the originating thread (which probably should have been posted here):

Perhaps the # symbol should not flag a warning since it can be used to reference other documents.

As to the bug, perhaps I am not fully understanding since I only scanned through. The Discard button works to discard the expression, thus the property or constraint is no longer linked. It is not a cancel button.

Which is in my opinion annoying, should have three buttons to save this confusion "accept", "cancel" and a "remove/delete expression".

Currently it's common that one looks at the expression, changes something or nothing, then if you change your mind and want just "cancel" the Expression edit widget to just go back to where you were before you started the expression edit/viewing....and you find you have instead deleted the entire Expression.

Does anyone see any objections to the above reasoning? If not, I'll file an enhancement request to change "Discard" to "Reset", and add a tooltip on "Reset" to state "Reset the values to the default", and optionally (and only if the screen real-estate can handle it) add an additional "Cancel" button.

Thanks to all for reading the above lengthy post!
-bg

[chrisb]

Sorry to vote against "Reset". Resetting a dialog has the common meaning of "set all values to their initial state and keep the dialog open." There is e.g. a "Reset" button in HTML forms which does exactly that. jmaustpc's proposal seems the most clear to me.

[bgoodr]

Sorry to vote against "Reset". Resetting a dialog has the common meaning of "set all values to their initial state and keep the dialog open." There is e.g. a "Reset" button in HTML forms which does exactly that. jmaustpc's proposal seems the most clear to me.

I agree with that. How about this? :

Change "Discard" to "Clear", and add a tooltip on "Clear" to state "Clear the expression", and optionally (and only if the screen real-estate can handle it) add an additional "Cancel" button. "Delete" can be used instead of "Clear".

[chrisb]

Clear/Delete or even more explicit "Clear/Delete expression" is clear (!) to me.

[bgoodr]

Got it.

I'll post the ticket now and leave it up to whomever to decide on the wording based upon how many characters they can put into the button, or limit it based upon size limitations etc.

[openBrain]

I admit that I also get confused as (as a non native english speaker), it was unclear if this button will discard my changes in the expression, or the expression binding itself.
"Clear" seems to be a very good choice. I'd also be OK with something like "Wipe off" but maybe less universally understood.

I'll post the ticket now

Maybe we could directly submit a PR. I had a quick check, but I guess it is a single change on this line