[Payment Request] Selector view edit mode

[Payment Request] Selector view edit mode

- Adds a trailing edit button to the navigation bar if the selector view
  supports edit mode.
- Tapping the edit button causes the selector view to enter the edit mode.
- Tapping the edit button again, causes the selector view to leave the
  edit mode.
- Tapping an item in the edit mode presents the editor view.
- Completing the edit dismisses the editor view updates the current
  selector view.

http://imgur.com/a/kf5Dp

BUG=602666
Review-Url: https://codereview.chromium.org/2938673003
Cr-Commit-Position: refs/heads/master@{#479403}
Committed: https://chromium.googlesource.com/chromium/src/+/c8ab74988779f64ba38116635b6b7b34d816b44d

[Moe, 2017-06-13 21:08:40]

The CQ bit was checked by mahmadi@chromium.org to run a CQ dry run

[Moe, 2017-06-13 21:08:53]

Description was changed from

==========
[Payment Request] Selector view edit mode

- Adds a trailing eidt button to the navigation bar if the selector view
  supports edit mode.
- Tapping the edit button causes the selector view to enter the edit mode.
- Tapping the edit button again, causes the selector view to leave the
  edit mode.
- Tapping an item in the edit mode presents the editor view.
- Completing the edit dismisses the editor view updates the current
  selector view.

BUG=602666
==========

to

==========
[Payment Request] Selector view edit mode

- Adds a trailing eidt button to the navigation bar if the selector view
  supports edit mode.
- Tapping the edit button causes the selector view to enter the edit mode.
- Tapping the edit button again, causes the selector view to leave the
  edit mode.
- Tapping an item in the edit mode presents the editor view.
- Completing the edit dismisses the editor view updates the current
  selector view.

http://imgur.com/a/kf5Dp

BUG=602666
==========

[Moe, 2017-06-13 21:08:53]

mahmadi@chromium.org changed reviewers:
+ lpromero@chromium.org

[commit-bot: I haz the power, 2017-06-13 21:09:20]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Moe, 2017-06-13 21:09:49]

Hi Louis, Please take a look. I will update the showcase eg tests and the unit
tests in a follow up CL.

[commit-bot: I haz the power, 2017-06-13 21:35:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-13 21:35:02]

Dry run: This issue passed the CQ dry run.

[lpromero, 2017-06-14 08:47:10]

Description was changed from

==========
[Payment Request] Selector view edit mode

- Adds a trailing eidt button to the navigation bar if the selector view
  supports edit mode.
- Tapping the edit button causes the selector view to enter the edit mode.
- Tapping the edit button again, causes the selector view to leave the
  edit mode.
- Tapping an item in the edit mode presents the editor view.
- Completing the edit dismisses the editor view updates the current
  selector view.

http://imgur.com/a/kf5Dp

BUG=602666
==========

to

==========
[Payment Request] Selector view edit mode

- Adds a trailing edit button to the navigation bar if the selector view
  supports edit mode.
- Tapping the edit button causes the selector view to enter the edit mode.
- Tapping the edit button again, causes the selector view to leave the
  edit mode.
- Tapping an item in the edit mode presents the editor view.
- Completing the edit dismisses the editor view updates the current
  selector view.

http://imgur.com/a/kf5Dp

BUG=602666
==========

[lpromero, 2017-06-14 09:10:13]

lgtm

Thanks for the video in the description, very useful.

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
File ios/chrome/browser/ui/payments/payment_request_selector_view_controller.h
(right):

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
ios/chrome/browser/ui/payments/payment_request_selector_view_controller.h:37:
didChangeToState:(PaymentRequestSelectorState)state;
The implementation doesn't seem to notify when changing to the other states than
Edit and Normal.
Should the new Edit/Normal state be considered orthogonal entirely?

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
File ios/chrome/browser/ui/payments/payment_request_selector_view_controller.mm
(right):

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
ios/chrome/browser/ui/payments/payment_request_selector_view_controller.mm:116:
if ([self.dataSource allowsEditMode]) {
If !allowsEditMode, should you clear any present button?
self.navigationItem.rightBarButtonItem = nil for example?

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
ios/chrome/browser/ui/payments/payment_request_selector_view_controller.mm:251:
:)]) {
No action needed: this line is awesome :p

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
File ios/chrome/browser/ui/payments/shipping_option_selection_mediator.mm
(right):

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
ios/chrome/browser/ui/payments/shipping_option_selection_mediator.mm:33: -
(void)createItems;
Optional nit: "loadItems" might convey a better semantic.

[Moe, 2017-06-14 14:33:50]

The CQ bit was checked by mahmadi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-14 14:34:20]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Moe, 2017-06-14 14:35:06]

Thank you Louis!

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
File ios/chrome/browser/ui/payments/payment_request_selector_view_controller.h
(right):

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
ios/chrome/browser/ui/payments/payment_request_selector_view_controller.h:37:
didChangeToState:(PaymentRequestSelectorState)state;
On 2017/06/14 at 09:10:12, lpromero wrote:
> The implementation doesn't seem to notify when changing to the other states
than Edit and Normal.
> Should the new Edit/Normal state be considered orthogonal entirely?

Done. You certainly have a point. Using the PaymentRequestSelectorState for the
edit mode guaranteed that the edit mode can't happen at the same time as the
pending or the error states. But it creates this discrepancy in when the view
controller does or doesn't notify the coordinator of the state change.  
I think its more appropriate to have an internal boolean state to manage the
edit mode and make sure the edit button only appears in the normal state to
begin with.

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
File ios/chrome/browser/ui/payments/payment_request_selector_view_controller.mm
(right):

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
ios/chrome/browser/ui/payments/payment_request_selector_view_controller.mm:116:
if ([self.dataSource allowsEditMode]) {
On 2017/06/14 at 09:10:12, lpromero wrote:
> If !allowsEditMode, should you clear any present button?
self.navigationItem.rightBarButtonItem = nil for example?

allowsEditMode shouldn't change during the lifetime of the view controller, but
clearing the button is definitely safer. Done.

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
ios/chrome/browser/ui/payments/payment_request_selector_view_controller.mm:251:
:)]) {
On 2017/06/14 at 09:10:13, lpromero wrote:
> No action needed: this line is awesome :p

:)])

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
File ios/chrome/browser/ui/payments/shipping_option_selection_mediator.mm
(right):

https://codereview.chromium.org/2938673003/diff/1/ios/chrome/browser/ui/payme...
ios/chrome/browser/ui/payments/shipping_option_selection_mediator.mm:33: -
(void)createItems;
On 2017/06/14 at 09:10:13, lpromero wrote:
> Optional nit: "loadItems" might convey a better semantic.

Done.

[lpromero, 2017-06-14 14:45:56]

LGTM. Thanks!

[commit-bot: I haz the power, 2017-06-14 15:13:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-14 15:13:39]

Dry run: This issue passed the CQ dry run.

[Moe, 2017-06-14 15:57:20]

The CQ bit was checked by mahmadi@chromium.org

[commit-bot: I haz the power, 2017-06-14 15:57:44]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-14 16:03:35]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497455840565890,
"parent_rev": "2a36d1978df99e98cf8696c5824bb8050f514928", "commit_rev":
"c8ab74988779f64ba38116635b6b7b34d816b44d"}

[commit-bot: I haz the power, 2017-06-14 16:03:46]

Description was changed from

==========
[Payment Request] Selector view edit mode

- Adds a trailing edit button to the navigation bar if the selector view
  supports edit mode.
- Tapping the edit button causes the selector view to enter the edit mode.
- Tapping the edit button again, causes the selector view to leave the
  edit mode.
- Tapping an item in the edit mode presents the editor view.
- Completing the edit dismisses the editor view updates the current
  selector view.

http://imgur.com/a/kf5Dp

BUG=602666
==========

to

==========
[Payment Request] Selector view edit mode

- Adds a trailing edit button to the navigation bar if the selector view
  supports edit mode.
- Tapping the edit button causes the selector view to enter the edit mode.
- Tapping the edit button again, causes the selector view to leave the
  edit mode.
- Tapping an item in the edit mode presents the editor view.
- Completing the edit dismisses the editor view updates the current
  selector view.

http://imgur.com/a/kf5Dp

BUG=602666

Review-Url: https://codereview.chromium.org/2938673003
Cr-Commit-Position: refs/heads/master@{#479403}
Committed:
https://chromium.googlesource.com/chromium/src/+/c8ab74988779f64ba38116635b6b...
==========

[commit-bot: I haz the power, 2017-06-14 16:03:48]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/c8ab74988779f64ba38116635b6b...