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

[NT-228, NT-376] Cancel Pledge Error Handling #891

Merged
merged 7 commits into from
Oct 15, 2019
Merged

Conversation

ifbarrera
Copy link
Contributor

@ifbarrera ifbarrera commented Oct 11, 2019

📲 What

Adds error handling via the message banner for the cancel pledge flow. This PR also adds the missing accessibility label on the cancellation reason text field, and updates the disabled and highlighted colors on the "Yes, cancel it" button.

🤔 Why

To let users know when cancelling their pledge has failed.

🛠 How

  • adds MessageBannerPresenting conformance to CancelPledgeViewController
  • hooks the banner up to the cancelPledgeError output
  • adds an accessibility label to the cancellationReasonTextField
  • updates the .disabled and .highlighted colors on the redButtonStyle

👀 See

tTUywwXmxU

♿️ Accessibility

  • The cancellation reason text field should read "Cancellation reason, Tell us why, optional" with voice over enabled

✅ Acceptance criteria

  • Navigate to a backing for a project that was not created on staging (any backing really). Navigate to the cancel pledge screen and try to pledge by tapping "Yes, cancel it". The button should become disabled and you should see an error banner animate in.

Copy link
Contributor

@cdolm92 cdolm92 left a comment

Choose a reason for hiding this comment

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

Looking good, just have a minor suggestion and found something unusual going on with the Japanese strings translations.

@@ -60,6 +60,7 @@
"Cancel" = "キャンセル";
"Cancel_pledge" = "プレッジを取り消す";
"Cancel_your_pledge" = "プレッジを取り消す";
"Cancellation_reason" = "Motivo de cancelación";
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are the Japanese strings in Spanish? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question...

Comment on lines 128 to 129
|> UITextField.lens.accessibilityLabel %~ { _ in Strings.Cancellation_reason() }
|> UITextField.lens.placeholder %~ { _ in Strings.Tell_us_why_optional() }
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get rid of the lens and do |> \.accessibilityLabel %~ { _ in Strings.Cancellation_reason() } and |> \.placeholder %~ { _ in Strings.Tell_us_why_optional() } here?

- **de**: "Grund der Annullierung"
- **es**: "Motivo de cancelación"
- **fr**: "Motif d'annulation"
- **ja**: "Motivo de cancelación"
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too. Something went wrong with the Japanese translations.

Copy link
Contributor

@cdolm92 cdolm92 left a comment

Choose a reason for hiding this comment

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

Thanks for doing this it's G2G!! 🛳🛳🛳

@ifbarrera ifbarrera merged commit 4d58861 into master Oct 15, 2019
@ifbarrera ifbarrera deleted the cancel-pledge-error branch October 15, 2019 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants