Save As Popup

[iamutkarshtiwari]

'Save As' popup added which appears on activity close. It allows user to provide instance name to the user. - https://www.youtube.com/watch?v=xcvBH7zzFBo

[icarito]

Allright I just tested this feature. My comments:

• [x] Text entry is way too small.
• [x] Text entry does not offer current object title for changing
• [x] There should be a way to cancel the alert.
• [x] It should not be possible to enter an empty name.
• [ ] Save as/Discard button is ambiguous (and it takes a lot of time to act for some reason).
• [ ] "Same name" warning is ambiguous. The same name object is the object itself.
• [ ] Overwrite action should happen if user insists with stop button.

Some serious thought needs to go into this interaction, as it is a very important one.

We must take into consideration that Sugar already saves the object if the user changed activities or views before, even if it was the first instance (of object creation).

Perhaps the alert could work this way: It could have default actions for each question. Pressing Stop button again will simply take default actions (I instinctively tried this, i.e. telling the system I really want to stop, don't ask me questions).

Take this as an idea:

• Confirm saving as: [Object title] - _save_, cancel
  • If a user changes the object title, the buttons could switch to _rename_, save copy, cancel
  • In addition, a menu button could be added for the following secondary actions: discard, **save to...***

[samdroid-apps]

I agree with @icarito about the issues with the current implementation.

Here are a few of my takes on it:

• What does the disregard button do? Is it suppose to delete the entry? Isn't that what was discussed on the mailing list? When I tested, the disregard button just did nothing and closed the activity :(
• Disregard is such a fancy word. Could we use more direct words like "save" and "delete" rather than "confirm" and "disregard"?
• Can it be a popwindow? Then we get the full width of the screen, and you can put the full journal details view there - making it easy to add a description. Using the full details view (rather than making a new view) is nice from a consistency perspective - it is a view that users are familiar with.
• Can the options be "save" and "delete". Save will save the journal object, with whatever the title and metadata the user has set.

[iamutkarshtiwari]

@icarito @samdroid-apps I have fixed some of the issues. Please test the newest commit.

@samdroid-apps When the user selects the discard option, the activity data (for eg. text in case of Write activity) doesn't get saved on close. Only the object entry appears in datastore for statistical purpose but not the activity data.

[samdroid-apps]

@samdroid-apps When the user selects the discard option, the activity data (for eg. text in case of Write activity) doesn't get saved on close. Only the object entry appears in datastore for statistical purpose but not the activity data.

I thought that one of the main reasons to add this change was the delete journal clutter by letting the user delete entries more quickly.

What is the use case of saving the metadata but not the entry data?

[iamutkarshtiwari]

@samdroid-apps Mr. Anderson asked me to build it that way.

[icarito]

"When the user selects the discard option, the activity data (for eg. text in case of Write activity) doesn't get saved on close."

But it's already saved before close! You mean it gets deleted?

[iamutkarshtiwari]

icarito Yes. When the activity is open, a duplicate copy is created. If the user selects to 'discard' the data, the duplicate copy is deleted without saving the data. And if the user selects to save the data, the duplicate copy is retained by the new name provided by the user in the text entry and the data is saved to that dupicate instance without overwriting the original instance.

[icarito]

Here's comments by Tony on private thread: "Consider:

You open Paint and create a red X and save it as x. Then you open x and change the image to a blue O. You want to save it as O, so you change the name. Now try to open X. It is gone.

Suppose you open Paint to show someone how to select a brush and give it a color. You scribble something. When asked for a name, you click on 'quit' because there is not need to save this image.

The alert should be simple:

Please give your project a title. [Untitled] Save Quit

The default title is 'Untitled' not 'Write.activity'. If the user clicks save with the title unchanged, it is ignored. If the user clicks on Quit, the activity quits (with no save of the Document - but the metadata is saved). if the user resumes an activity the alert shows:

Change the title to save as a new document [X] Save Quit

The user changes the title [X] to [Y]. Then Save saves Y to the Journal and X is still in the Journal as well. If the user doesn't change and clicks Save, the X object is deleted and the new X version is saved. if the user clicks on Quit - the document is not saved, the original X document is in the Journal and the activity quits."

I can't find Tony's github user to mantion him.

My comment is:
Clicking Stop, and then "Quit" does not tell me as a user that my work will be deleted (this is what you seem to be suggesting).

"Change the title to save as a new document": seems long to me. Also it's not always a document (e.g. Browse session).

if the user clicks on Quit - the document is not saved, the original X document is in the Journal and the activity quits." - but the document may already have been saved (on view change, for instance).

[icarito]

"If the user selects to 'discard' the data, the duplicate copy is deleted without saving the data. "

This is the part that I don't understand, Sugar will save the data on every view change. So you mean it will delete the duplicate (that may have been previously already saved).

[icarito]

I tried the new version and it works much better. I'm not convinced by the text in the alert. You seem to imply two objects can't have the same name, but they can. Objects are identified by a unique hash id in the journal.

Save as / Quit is a strange button label.

One of Sugar's design principles is simplicity.

[icarito]

Currently my main concern from a UI pov is that choosing "Stop" and then "Quit" will result in the user discarding his work. This is not intuitive to me.

[icarito]

"Mr. Anderson asked me to build it that way." - That's a bad reason, you should believe in what you do! There are better ways to collect metadata than to clutter the journal. This has been a bad use of it.

[iamutkarshtiwari]

@icarito I agree. But the 'SaveAs' feature was proposed by Mr. Anderson and I'll have to abide by the requirements he specifies.

[tony37]

This thread would be a lot more useful if you took the time to understand Sugar as well as this PR.

"Mr. Anderson asked me to build it that way." - That's a bad reason, you should believe in what you do! There are better ways to collect metadata than to clutter the journal. This has been a bad use of it.

This is crazy. I am mentoring this feature. Granted Utkarsh can proceed as he feels best but we don't need to involve him in these quibbles.

Any Journal object consists of two elements: a metadata file and an (optional) data file. The data file normally conforms to a meta_type and is from the point of view of the user, a document.

The metadata needs to be saved in every case. The document only if the user chooses. Note: that any user is free to delete any Journal object at any time without getting permission from anyone.

[tony37]

Currently my main concern from a UI pov is that choosing "Stop" and then "Quit" will result in the user discarding his work. This is not intuitive to me.

This is why you should discuss the issue. The alert offers two options: save and quit. The alert appears because the user has clicked on 'stop' to request terminating the activity.

The current activity.py does indeed save the document without action by the user (we used to have 'keep' which gave the user the option to save and then continue. This was removed.

However, the implementation is flawed. The jobject used by activity.py in case of resume is the original jobject_id. This means, unknown to the user, he is making irretrievable changes to his document. Hence, the scenario quoted by Utkarsh.

Utkarsh makes a copy of the original jobject and this is what is changed. This enables the save option after a name change, to keep both the original document and the new instance as a user should expect.

[tony37]

When choosing to overwrite do you delete the previous copy or do you always keep a copy?

If you take the trouble to read the description of the feature, you will know that if the user resumes an activity with a title and chooses not to change the title, then the modified document is saved and the original is deleted. This is exactly what happens now. If the user changes the name, then the new document is saved under its new name and the unmodified original is also in the Journal.

[tony37]

If you have cosmetic changes make them on another PR. It makes it hard to see what you changed

Who decides what changes are important and which are 'cosmetic'. This is the PR.

[tony37]

The reason 'ImageViewer' and 'Chat' didn't close is because these two activities override the 'can_close()' funtion of 'activitly.py' which is responsible to display my alert.

I have figured out a solution and commit a new tweak for the same.

Utkarsh: 'can_close' is a function which can be supplied by the activity. In that case, the copy in activity.py is not called. What you need to do is post the alert after can_close. This way from the user's view, the alert will be seen before close. When the user responds to the alert, the activity can close.

[tony37]

I tried the new version and it works much better. I'm not convinced by the text in the alert. You seem to imply two objects can't have the same name, but they can. Objects are identified by a unique hash id in the journal.

From the user perspective, documents should have a unique name. Granted in standard software, different files may have the same name in different directories. We only have the Journal.

Save as / Quit is a strange button label.

The button options should be 'save' and 'quit'. The text on the button may differ from the start new case when the title is 'Untitled' or 'Write.activity' - perhaps: Please give a name to your project. In the resume case, the text should refer to the option to save (no change) or save as (change). Perhaps, Change the name of your project to save a new document.

One of Sugar's design principles is simplicity.

[iamutkarshtiwari]

@icarito @tony37 The 'Chat' and 'ImageViewer' deadlock on close issue is resolved in the latest commit. Please test it and share your suggestions

@tony37 We all together can workout this feature's design and functionality :) After all, our contributions are going to benefit our Sugar community.

[icarito]

As promised here's a screenshot of Sugar 0.92's "save as" dialog:

As per our meeting today I think Martin Dengler's proposal to land the user in the Journal after quitting with focus on Object Title would be a good solution.

[icarito]

Correction screenshot is from Sugar 0.90.2

[tony37]

As you might expect, I totally disagree.

First, by that time the damage has been done. Either the 'save as' option has been taken with a new object created or the 'resumed' document has been overwritten - as at present.

This feature has two objectives:

 1 - require Journal items to have a name supplied by user
 2 - enable the user to choose whether to save the object as a new 

one or to simply save the modified document.

This proposal addresses neither objective.

You are not going to change your point of view so the only solution I can see is to provide a gsetting option so that users and deployments can decide on how they want this to work.

Re: the screenshot. Help me - as I remember this dialog was modal and required an entry in the description area - leaving the title as 'Write activity' was ok. This was to implement Walter's desire that the description serve as the message does in git.

Tony

On 07/15/2016 04:36 AM, Sebastian Silva wrote:

As promised here's a screenshot of Sugar 0.92's "save as" dialog: captura de pantalla de 2016-07-14 21-33-17 https://cloud.githubusercontent.com/assets/199755/16861909/cc262c7c-4a0a-11e6-8ebd-88c4a5157595.png

As per our meeting today I think Martin Dengler's proposal to land the user in the Journal after quitting with focus on Object Title would be a good solution.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/327#issuecomment-232845789, or mute the thread https://github.com/notifications/unsubscribe-auth/AAULkhTztLdY3u833M4V3dte_sV3q3RMks5qVvISgaJpZM4JEg63.

[tony37]

I think user testing can resolve such ux disagreements.

A setting to choose from both is not a wise design decision because there is still a question of which is default and how easy it is to set.

Since Tony is advocating for a change, perhaps you could do a user test. Have you done one before? :)

[samdroid-apps]

This feature has two objectives:

1 - require Journal items to have a name supplied by user
2 - enable the user to choose whether to save the object as a new

one or to simply save the modified document.

This proposal addresses neither objective.

I think that user testing is invaluable there. But I think that @icarito very clearly addresses item number 1 - the proposal literally puts the user in front of a screen dedicated to managing activity metadata.

For addressing proposal 2, did you want to make https://github.com/sugarlabs/sugar/pull/699 false by default? I would support making that false by default, now that I understand the rational.

But seriously, if you do some user testing and write up some scenarios for the users, I can also find some other people and carry out the same tests too! More testing the better!

[tony37]

Sam

I don't understand you. #327 satisfies both objectives.

"The proposal literally puts the user in front of a screen dedicated to managing activity metadata." Clearly, this must be something completely different from $327 or #699. Perhaps you could describe this proposal in more detail. Does it have a PR? Could you be referring to Sebastian's suggestion to switch the user to the Journal view on close?

699 is a completely different feature. Yes, it should not change Sugar

unless the user or deployment chooses. While I many deployments have asked for this capability, it is not consistent with Walter's decision to provide resume from the home view as the default behavior for Sugar.

I have no idea how to test the first item you describe - it has nothing to do with either #327 or #699. I have had 'resume' implemented at least 5 schools with nearly 500 XOs operating Sugar. I am not sure what more user experience is needed. Remember, this behavior was the way Sugar worked through its intial releases and may, for all we know, be the way it still works on the majority of operational XOs.

User tests on #327 seem unnecessary, We all use it everyday on Linux, Windows, or Mac. My most common use is on nano where when I close a buffer, I must give it a name. If it has a name, I am given the opportunity to change it. Virtually all document-handling applications that I am aware of work this way.

Tony

On 07/15/2016 11:54 PM, Sam wrote:

This feature has two objectives:

|1 - require Journal items to have a name supplied by user 2 -
enable the user to choose whether to save the object as a new |

one or to simply save the modified document.

This proposal addresses neither objective.

I think that user testing is invaluable there. But I think that @icarito https://github.com/icarito very clearly addresses item number 1 - the proposal literally puts the user in front of a screen dedicated to managing activity metadata.

For addressing proposal 2, did you want to make sugarlabs/sugar#699 https://github.com/sugarlabs/sugar/pull/699 false by default? I would support making that false by default, now that I understand the rational.

But seriously, if you do some user testing and write up some scenarios for the users, I can also find some other people and carry out the same tests too! More testing the better!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/327#issuecomment-233079108, or mute the thread https://github.com/notifications/unsubscribe-auth/AAULkogxGP2httu5vM9EFwM_XFC_9KgPks5qWAGagaJpZM4JEg63.

[iamutkarshtiwari]

Here are the latest changes -

[davelab6]

Love that GIF :)

Tony, I think user testing each change is always a good idea, if only to validate that your assumption the change is for the better is true :)

[tony37]

Hi, Dave

We can agree to disagree. In the first place, as Sam Parkinson found out, finding users representative of our target users is hard. Children in the Western world have experience with computers and smart phones and so on.

Second, I am unaware of user testing for most of the changes made to Sugar. For example a check box was added to the Journal View. How was that user tested?

We have trouble even getting changes documented for our users which is certainly required.

Tony

On 07/17/2016 08:43 PM, Dave Crossland wrote:

Love that GIF :)

Tony, I think user testing each change is always a good idea, if only to validate that your assumption the change is for the better is true :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/327#issuecomment-233197005, or mute the thread https://github.com/notifications/unsubscribe-auth/AAULkhT54Z27TSq0a-_B0Ac_xK1UmQetks5qWne3gaJpZM4JEg63.

[samdroid-apps]

Hi, Dave

We can agree to disagree. In the first place, as Sam Parkinson found out, finding users representative of our target users is hard. Children in the Western world have experience with computers and smart phones and so on.

I never remember finding that out.

I want Sugar to be a great piece of software - one that even kids with experience of other computers and smart phones would choose.

I thought that we wanted to have an inclusive target market, of both kids who are the first computer owners in their family, and kids who are Born Mobile.

Second, I am unaware of user testing for most of the changes made to Sugar. For example a check box was added to the Journal View. How was that user tested?

Good, let's change that! Let's start at the Open Source Usability blog: http://opensource-usability.blogspot.com.au/2012/11/how-many-testers-do-you-need.html

[tony37]

Hi, Sam

Let's start with the project view in the Journal.

As I remember, you reported failure in your user test of 'on-boarding'. As I also recall, the users tested felt that the 'on-board' feature held them up because they already knew how to do it. This is unlikely to be the reaction of our deployed users who have much more limited computer experience. If you want to test Sugar features with kids 'born mobile' great. This is particularly important if the question you are asking is would this feature attract new Sugar users from the Western world who are blessed with access to computers and to high-speed internet 24/7.

Tony

On 07/18/2016 01:30 PM, Sam wrote:

Hi, Dave

We can agree to disagree. In the first place, as Sam Parkinson found
out, finding users representative of our target users is hard.
Children
in the Western world have experience with computers and smart
phones and
so on.

I never remember finding that out.

I want Sugar to be a great piece of software - one that even kids with experience of other computers and smart phones would /choose/.

I thought that we wanted to have an inclusive target market, of both kids who are the first computer owners in their family, and kids who are Born Mobile https://www.youtube.com/watch?v=p1w2IxMWQ1Y.

Second, I am unaware of user testing for most of the changes made to
Sugar. For example a check box was added to the Journal View. How was
that user tested?

Good, let's change that! Let's start at the Open Source Usability blog: http://opensource-usability.blogspot.com.au/2012/11/how-many-testers-do-you-need.html

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/327#issuecomment-233304662, or mute the thread https://github.com/notifications/unsubscribe-auth/AAULko7y7sFIAIQKfJmg-JO7SYxTeNYxks5qW2PKgaJpZM4JEg63.

[iamutkarshtiwari]

I have generated a similar PR for sugar-toolkit too. https://github.com/sugarlabs/sugar-toolkit/pull/4

[iamutkarshtiwari]

@samdroid-apps @icarito @davelab6 Please test this PR and share your reviews. I have simplified the popup's UI as suggested.

[quozl]

Reviewed. I'm fine with this if the new behaviour can be disabled using gsettings. Several deployments who work with OLPC won't be interested in returning to the 0.92 save-as dialog days, so I'll need a way to turn it off.

[i5o]

Added to 0.112 milestone; we should re-discuss this.

[tony37]

It works. I am installing in 13.2.8 for use in Rwanda. Adding a gsetting should be simple, This has already been done for the 'resume-activity' feature which has been enabled by setting resume-activity to false. The screen shot feature PR675 should also be implemented with a gsetting control.

What needs to be discussed is how to expose these and other gsetting options to users and deployments. Perhaps something like about:config.

[iamutkarshtiwari]

@i5o @tony37 This patch is working fine (tested well). I agree with Mr. Anderson. We can add a gsetting for enabling/disabling this feature in Sugar, something like - gsettings set org.sugarlabs.user save-as true

@davelab6 @quozl @samdroid-apps What do you think?

[i5o]

The question is, will it be enabled as default?

On 1/2/17, Utkarsh Tiwari notifications@github.com wrote:

@i5o @tony37 This patch is working fine (tested well). I agree with Mr. Anderson. We can add a gsetting for enabling/disabling this feature in Sugar, something like - gsettings set org.sugarlabs.user save-as true

@davelab6 @quozl @samdroid-apps What do you think?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/327#issuecomment-269945857

-- Ignacio Rodríguez

[iamutkarshtiwari]

@i5o Let's set it to disabled by default. Should any user be interested in exploring the feature, they can set it enabled from gsettings option

[quozl]

If nobody is willing to drive this feature to completion following the consensus above, then I'd say momentum is lost and it would be better to close the pull request until there is interest again. I'll close it in a few days if there is no material progress.

[tony37]

What's to complete. The request is to merge the Pull Request. The only apparent open issue is whether this feature should be enabled by a gsetting with the default disabled.

That seems the correct strategy preserving compatibility for the user unless they know what they want.

While the above statement is that there is no conflict, installation on 0.110 results in several activities such as TurtleBlocks crashing. This needs to be investigated, of course.

I do not understand the comment about loss of momentum. This PR was developed as part of GSOC16. The work was completed and presented for merger. We are awaiting action by the responsible developers.

[tony37]

I am unfamiliar with the technical term 'momentum'. The request is clear, merge the changes with the master. This PR was developed as part of GSOC16 so any momentum, whatever that is, is the responsibility of the developers.

The material progress needed is to test the PR with the current repository since it is presented as a patch against an unspecified commit. The action is waiting for the responsible developers to act on the request to merge this PR and has been for months.

It is proposed that this feature be made disabled by default and to be enabled by a gsetting (as was done with the resume PR). This is appropriate preserving current behavior but making it available to those that want it.

Tony

On 04/22/2017 06:10 AM, James Cameron wrote:

If nobody is willing to drive this feature to completion following the consensus above, then I'd say momentum is lost and it would be better to close the pull request until there is interest again. I'll close it in a few days if there is no material progress.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/327#issuecomment-296317109, or mute the thread https://github.com/notifications/unsubscribe-auth/AAULkidnrygkOIllltsPfSEEjTbK_2uxks5rySlMgaJpZM4JEg63.

[quozl]

Waiting for the pull request to;

• [ ] add a gsetting, with a default,
• [ ] use the gsetting,
• [ ] resolve reported incompatibility with TurtleBlocks,

And since I'm quite surprised that it had an impact of activities, I'd also suggest;

• [ ] evidence of testing with other activities.

[tony37]

Hi, James

This PR worked with 0.108. My belief is that changes to the files involved after 0.108 conflicted with the patches. Since two of the files affected are sugar/activity.py and sugar3/activity.py, an impact on activities of a faulty merge seem inevitable.

Since some unknown person is responsible to merge PRs, presumably that person knows how to tell when the merge conflicts. Apparently there is nothing that the developer of a PR can do about this.

Tony

On 04/22/2017 10:54 AM, James Cameron wrote:

Waiting for the pull request to;

• add a gsetting, with a default,
• use the gsetting,
• resolve reported incompatibility with TurtleBlocks,

And since I'm quite surprised that it had an impact of activities, I'd also suggest;

• evidence of testing with other activities.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/327#issuecomment-296342339, or mute the thread https://github.com/notifications/unsubscribe-auth/AAULkmOxpX17w1llO86Tj9L1sY-AmO1Vks5ryWvsgaJpZM4JEg63.

[quozl]

Developer of a pull request is jointly responsible with all other developers to resolve bugs and merge conflicts. There is no merge conflict at the moment between this pull request and the master branch.

We're working toward 0.112 now, with 0.110 released some time ago, and 0.108 now quite old; so that it worked with 0.108 and TurtleBlocks is of diagnostic use to whomever will be looking into the bug.

I've no change to what I'm waiting for in my reply above.

[tony37]

The 'no conflicts' does not assure that the merge meets the intention of the developer. What is considered the base branch in this case?

I interpret 'jointly' to mean co-operation not finger-pointing.

[quozl]

I don't understand why you want to know the base branch for merges, but it is master.

[tony37]

Because master today may not be the same file as that used to construct the patch.

Tony

On 04/22/2017 12:08 PM, James Cameron wrote:

I don't understand why you want to know the base branch for merges, but it is /master/.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/327#issuecomment-296345675, or mute the thread https://github.com/notifications/unsubscribe-auth/AAULksmSyHADR-3rwLKGXs6vnmqx9gHXks5ryX07gaJpZM4JEg63.

[quozl]

Yes, of course, but it is master that the merge must be made to. Nobody is interested in a patch to 0.108 to add a feature in the absence of all the subsequent patches that fix other problems. Still, there are no merge conflicts as far as git is concerned; no other conflicting changes have been made to the code that this pull request touches. Any problems you have with the four commits will be unrelated to a merge conflict.

[tony37]

It is clear that no help with this is forthcoming. I'll try to see if this can be straightened out so that the PR can be merged (clearly no one believes the no conflict message and is willing to just automatically merge - me included).

Tony

On 04/22/2017 02:22 PM, James Cameron wrote:

Yes, of course, but it is /master/ that the merge must be made to. Nobody is interested in a patch to 0.108 to add a feature in the absence of all the subsequent patches that fix other problems. Still, there are no /merge conflicts/ as far as |git| is concerned; no other conflicting changes have been made to the code that this pull request touches. Any problems you have with the four commits will be unrelated to a /merge conflict/.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/327#issuecomment-296351706, or mute the thread https://github.com/notifications/unsubscribe-auth/AAULkplUmPRomdfttaw89oP942tKlrU2ks5ryZycgaJpZM4JEg63.