Adapt the SaveVersionDialog text field to use ctr+s for accept #268
Conversation
|
Oh, how could that change in the postscript have come to be? I created this commit using the latest develop version of Squot and the commit dialog did not show changes to the postscript being staged. |
|
|
|
@LinqLover also has run into a postscript issue in #258 and I have as well, as can be seen here: 9254dfd, cb398d1 But I did not find the cause yet, at least if Monticello-ct.715 is not the only cause. Please open a separate issue and check the following: does the package currently have a postscript in your image (before the commit, after the commit)? |
|
Now on to the actual pull request: do you want to finish the commit by pressing ctrl+s in the text field, or is it something different? Because if you wanted to "double confirm" the message (first ctrl+s, then press commit) instead, I would not like this. |
| 'src\/Squit.package' : #SquotCypressCodeSerializer, | ||
| 'src\/BaselineOfSquot.package' : #SquotCypressCodeSerializer, | ||
| 'src\/SquotTonel-Core.package' : #SquotCypressCodeSerializer, | ||
| 'src\/SquotTonel-Tests.package' : #SquotCypressCodeSerializer |
There was a problem hiding this comment.
Looks like you have a different version of STON than I have?
| @@ -0,0 +1,5 @@ | |||
| accessing | |||
| message: anObject notifying: aController | |||
There was a problem hiding this comment.
Since aController is unused here, why change from message: to message:notifying:?
| "actionCommit" : "fn 4/11/2017 16:01", | ||
| "activateNodeCommandLabel" : "jr 4/7/2019 22:30", | ||
| "browseOtherEdition" : "jr 6/6/2020 01:04", | ||
| "browseOtherEditionLabel" : "jr 6/6/2020 00:48", | ||
| "buildButtonBar:" : "jr 11/18/2018 01:39", | ||
| "buildDiffPane:" : "jr 11/18/2018 01:33", | ||
| "cancel" : "jr 11/18/2018 00:44", |
There was a problem hiding this comment.
The text was updated even on cancel, so the text could be saved for later.
Without your change, one can start a commit, write a message, then notice a bug in the diff, cancel, fix the code, press commit again, and the previously typed message is filled in automatically.
That logic is not in the dialog class, but in the SquotWorkingCopy or SquotInteractiveSaveOperation.
| @@ -5,7 +5,8 @@ widgetSpecs: builder | |||
| name: 'message for the new version'; | |||
| model: self; | |||
| getText: #message; | |||
| setText: #message:; | |||
| setText: #message:notifying:; | |||
| askBeforeDiscardingEdits: true; | |||
There was a problem hiding this comment.
It takes me too long to check this myself in Morphic: does it prevent the commit dialog from closing if you press cancel? If so, how?
There was a problem hiding this comment.
Please check and make sure that
- the message is still remembered even if you close or cancel the dialog,
- pressing ctrl+s is equivalent to hitting the commit button,
- that pressing ctrl+s is an optional way to accept, not a required one. That means you must still be able to hit commit and the text that is currently in the box will be taken, even though it may not have been updated in the model yet.
Also think whether this is really something the user will expect: in workspaces you can press accept to just remember the text for later, but here it will actually start a transaction on the repository and close your window.
|
Hey Jakob, thank you for that detailed list of requirements, that's exactly what I was looking for :) I already kinda suspected that it was meant to persist the text across invocations but didn't manage to follow the control/data flow to make that happen. The intent was to have a keyboard-based way to finish a commit, so the behavior in my image right now is that I can either press the "Accept" button or hit ctrl+s to confirm and perform the transaction. This is consistent with all other dialog windows in Squeak where accept also means confirm. I suppose in this way you can also look at editing a method as a transaction. I will update the PR in a bit to address all issues. Sorry that it looks a bit messy, I only looked at the GitBrowser's diff before placing the PR and didn't notice the extra changes.
Could you tell me how to best find the postscript in my image? Would I inspect the artifacts in my the GitBrowser at the respective commits? The postload for Squot.core contains the script in both this PR's commit and its parent. |
|
"Loaded" scripts you find by right-clicking on the package in the object list in the Git Browser (bottom right). Or in the Monticello browser (working copy browser) select the package and use the Scripts button. Historical scripts you can find by selecting the commit, in the objects list choose "Explore" in the package in question, select the #artifact item in the explorer and inspect |
|
...or you do it much simpler: select the commit, right-click the package, and just choose "Browse edition in selected version" :-) In the Monticello snapshot browser you will see the scripts when you select nothing in the panes above. |
|
Alright I believe I got the behavior correct in all cases now. Instead of cancel, we now wait for windowIsClosing to also catch the user dismissing the window without clicking the Cancel button. I changed the spec's setter selector to acceptMessage: to make it clear that this will also commit the transaction. #message: is still used by the setup code that restores the previous message. How should I got about the inadvertent changes? Just reset them and do a cleanup commit? I am also not sure what happened on the STON output, it should be the latest version from the squeak-ston repo. |
|
...and the |
While this change works, I had to drop the method
updateMessageFromViewsas it would have send the accept message to the text field on cancel as well. Sadly, I did not understand what the idea behindupdateMessageFromViewswas, @j4yk could you maybe explain? I'm a bit afraid this removes functionality I was not aware of.