Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Restore Revision dialog iOS #12141

Closed
1 task
yaelirub opened this issue Jul 16, 2019 · 16 comments · Fixed by #12947
Closed
1 task

Implement Restore Revision dialog iOS #12141

yaelirub opened this issue Jul 16, 2019 · 16 comments · Fixed by #12947

Comments

@yaelirub
Copy link
Contributor

yaelirub commented Jul 16, 2019

For discovery history - wordpress-mobile/WordPress-Android#10008, wordpress-mobile/WordPress-Android#9812
If the post has no local changes and a remote autosave, we are going to show a dialog to the user.
This issue is for implementing the design for this dialog.
screenshot-2019-05-03-at-09 25 19

Matching Android Issue:

wordpress-mobile/WordPress-Android#10108

Known Tasks

###UPDATE - 10/21/19
the acceptance criteria has evolved through a couple of Android issues and pull requests:

  • iOS will use the system alert (pending design approval) for the dialog
  • Posts containing autosaves on the post list screen will display the label "You’ve made unsaved changes to this post", this label previously read "Unhandled Auto Save"
  • Even after tapping on the “Restore” button, users can still back out of the post without editing - and if they tap edit again they will be presented with the dialog (this will be done via the "Discard Changes" button, any unsaved changes are discarded)
@yaelirub yaelirub transferred this issue from wordpress-mobile/WordPress-Android Jul 16, 2019
@yaelirub yaelirub changed the title Design Restore Revision dialog iOS Implement Restore Revision dialog iOS Jul 16, 2019
@shiki shiki assigned shiki and unassigned shiki Sep 24, 2019
@shiki
Copy link
Member

shiki commented Sep 26, 2019

@guarani will be working on this.

@shiki
Copy link
Member

shiki commented Sep 30, 2019

Moved this out of the Offline Posting board to keep the board focused on “posting”.

@guarani
Copy link
Contributor

guarani commented Oct 20, 2019

@yaelirub, regarding #12141 (comment):

If the post has local changes and a remote autosave, we are going to show a dialog to the user.

[emphasis added]

After reading wordpress-mobile/WordPress-Android#9812 (comment), I'm wondering if the above comment should read:

"If the post has NO local changes and a remote autosave, we are going to show a dialog to the user."

I think this is what was done in Android:

https://github.com/wordpress-mobile/WordPress-Android/pull/10426/files#diff-bdb6ebf0ab007bf9820664cba5dd351bR462-R464

@shiki
Copy link
Member

shiki commented Oct 22, 2019

"If the post has NO local changes and a remote autosave, we are going to show a dialog to the user."

That's correct, @guarani.

@guarani
Copy link
Contributor

guarani commented Oct 22, 2019

@yaelirub, the acceptance criteria has evolved through a couple of Android issues and pull requests, so I'm just updating here for completeness:

  1. iOS will use the system alert (pending design approval) for the dialog
  2. Posts containing autosaves on the post list screen will display the label "You’ve made unsaved changes to this post", this label previously read "Unhandled Auto Save"
  3. Even after tapping on the “Restore” button, users can still back out of the post without editing - and if they tap edit again they will be presented with the dialog (this will be done via the "Discard Changes" button, any unsaved changes are discarded)

@yaelirub
Copy link
Contributor Author

Thanks, @guarani :)
Please feel free to update the issue description according to the update.

@guarani
Copy link
Contributor

guarani commented Oct 22, 2019

I don't seem to have edit permission on the issue description - is that something you could enable, @yaelirub?

@guarani
Copy link
Contributor

guarani commented Oct 22, 2019

I'm cross-posting a prototype for the dialog here for design approval:

prototype

For the same reasons we’re using a system dialog on Android, the proposal here is to use the system alert on iOS. I don’t have many customization options here – apart from adding empty lines, changing the button order, and marking one button as “destructive” – in that it will be an alternative color (e.g. red).
(The text size shrinks on the first button due to the length of the button title.)

(originally posted on P2) cc: @osullivanchris @mattmiklic

@yaelirub
Copy link
Contributor Author

@guarani , no worries. I updated the description.

@osullivanchris
Copy link

@guarani thanks for digging around and finding the other relevant tickets, particularly the Android issue. As we have a number of similar and related scenarios, it takes me some time to recall which is which when I encounter these issues again. However I can see that this is one I already worked through on Android with Jirka.

I am happy to go ahead with the proposal here, the only change I would ask is to change the button copy to Sentence case. The all caps styling is just an Android/Material thing.

One question I have is what happens if the text on those buttons gets too long? Is it truncated? I can see that the text is smaller in the longer string here, so maybe that's the solution. Is that part of the system component and a common pattern to use, in your opinion?

Thanks!

@guarani
Copy link
Contributor

guarani commented Oct 23, 2019

Thanks Chris, the sentence case escaped me - I'll use that.
I think the text shrinks up to a certain point, but if the text is too long it gets truncated. I can double check this behavior tonight.

Update: Confirmed that on iOS, button text does indeed truncate when it gets quite long:

Screen Shot 2019-10-23 at 10 44 35 PM

@mattmiklic
Copy link
Member

I have a hunch that the amount of text used for those buttons isn't going to work well in some other languages; they're not even working that well in English at the moment.

Could we try giving the two versions labels in the text of the alert, and then using those labels for the two buttons? The labels could be "From another device" or "From this device". We could also try "Previous" and "Most Recent". Or label them "Version A" and "Version B" -- anything that could make those labels less verbose.

@guarani
Copy link
Contributor

guarani commented Oct 24, 2019

@mattmiklic good point - it could vary a lot by language. Personally I find the "From another device"/"From this device" to be the most user friendly.
@osullivanchris, shall we go with one of these options on iOS?

@osullivanchris
Copy link

osullivanchris commented Oct 24, 2019

Ah tricky! For @mattmiklic's second option, I think its hard to come up with good labels. And its tricky because the user has to parse the information in the dialog and then match it back to the options in the buttons.

As I explored in option 2 here, I thought the way to make selection easier was to provide metadata and action next to each other, so the user can make a direction selection and not to need an abstract name. But as we've found its not supported by the native dialog implementation on either iOS or Android and would require too much customisation.

So I would go with the simpler suggestion from @mattmiklic just shortening it to "From another device" and "From this app" which at least avoids some truncation/localisation issues.

@mattmiklic
Copy link
Member

@osullivanchris what do you think about "From this device" instead of "From this app"? I'm thinking that the choice between "this device" / "another device" seems clearer than "this app" / "another device".

@osullivanchris
Copy link

I like it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants