Skip to content
This repository has been archived by the owner on Dec 14, 2021. It is now read-only.

Confirm dialog for edit screen #1059

Merged
merged 6 commits into from
Jun 28, 2019
Merged

Confirm dialog for edit screen #1059

merged 6 commits into from
Jun 28, 2019

Conversation

joeyg
Copy link
Contributor

@joeyg joeyg commented Jun 16, 2019

This is a PR against the edit-layout screen branch (Details here). It adds the discard changes dialog.

@joeyg joeyg requested a review from a team June 16, 2019 22:28
sashei
sashei reviewed Jun 18, 2019
View reviewed changes
Copy link
Contributor

@sashei sashei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a question about which scenarios we should show the Discard changes? dialog... not sure what the intention is there but I think we wanted to be more discriminating about it than displaying it every time.

lockbox-ios/Presenter/ItemDetailPresenter.swift Show resolved Hide resolved
lockbox-ios/Presenter/ItemDetailPresenter.swift Show resolved Hide resolved
@joeyg joeyg requested review from a team June 28, 2019 01:24
devinreams
devinreams approved these changes Jun 28, 2019
View reviewed changes
Copy link
Contributor

@devinreams devinreams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMG_41F0FEDB93BD-1

lockbox-ios/Presenter/ItemDetailPresenter.swift Show resolved Hide resolved
@devinreams
Copy link
Contributor

Works as expected, and code looks OK here. Thank you @joeyg! 🙇

I'll go ahead and merge this into the bigger layout PR #1060 so we can review the 'edit mode' a bit more holistically. 🌈

@devinreams devinreams merged commit a0c7fa6 into 937-edit-layout-jg Jun 28, 2019
@devinreams devinreams deleted the edit-confirm-dialog branch June 28, 2019 20:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sashei sashei sashei left review comments

@devinreams devinreams devinreams approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@joeyg @devinreams @sashei