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

Fix/remove cancel button #17

Closed
wants to merge 4 commits into from
Closed

Conversation

richardtop
Copy link
Contributor

Now, the only button is Done and if Picker is empty, it returns empty array.

@zenangst
Copy link
Contributor

@hyperoslo/myshop-ios we don't need the cancel button anymore?

@richardtop
Copy link
Contributor Author

@zenangst I've talked to @RamonGilabert before implementing this and we agreed to remove cancel button completely.

@gilstad
Copy link

gilstad commented Aug 26, 2015

@Richardoti @RamonGilabert Do you mean removing the "Done" button in the full screen image gallery?

@RamonGilabert
Copy link
Contributor

@gilstad you are everywhere! :) Well, Richard told me to just display Done instead of Cancel, since, when you select 4 images and you deselect them, you are also done for instance, you don't want to cancel I guess. The reason behind it was like that right @Richardoti?

@richardtop
Copy link
Contributor Author

@gilstad, no. I mean this "Cancel" button:
screen shot 2015-08-26 at 17 09 20

So, the bottom view of the screen will always look like this - "Done" instead of context-dependent "Done"/"Cancel"
ios simulator screen shot 26 aug 2015 17 09 01

The reason for this change, as @RamonGilabert pointed, because now it is impossible remove all the photos if user selected any. One photo will always be selected - because "Cancel" button does not apply changes, it just dismisses the screen. The only way to remove all photos - close and reopen composer.

With this fix changes are being applied always, so if you remove all the photos, you can press "Done" and no photos will be attached to the post.

The bug with 4 images is not related to this and fixed in #13

@gilstad
Copy link

gilstad commented Aug 26, 2015

@RamonGilabert I dont read everything on git in detail, but I try to get an overview of the things happening. If I see something that I think goes in the wrong direction I comment ;)

I don't see the reason for not allowing to remove all images. This should be possible. If the user don't want to add pictures anyway we should not force him to close the whole composer.

Se we should keep the two options the button in question

  • Before any picture is captured and if all pictures are deselected -> Cancel
  • When one or more picture is selected -> Done

@richardtop
Copy link
Contributor Author

@gilstad I agree with the idea of allowing user to deselect all the images - it is exact reason, why I did this pull request. Now it is impossible to do.

if all pictures are deselected -> Cancel - then user just dismisses the ImagePicker and does not do any changes, i.e. cannot remove the images as his action would be cancelled.

If we use only Done button - it is clear that all the changes will be saved. When user presses Done, all the selected images are being taken by the composer. If he presses Cancel - all changes are discarded -> composer has all the images that it had before.

I hope, it's clear, but if you want, I can show you the live demo of the case I am talking about.

@richardtop richardtop changed the title Fix/remove cancel button [wip]Fix/remove cancel button Aug 27, 2015
Conflicts:
	Source/ImagePickerController.swift
@richardtop richardtop changed the title [wip]Fix/remove cancel button Fix/remove cancel button Aug 28, 2015
@zenangst zenangst closed this Oct 1, 2015
@zenangst zenangst deleted the fix/remove-cancel-button branch October 1, 2015 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants