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-526] Max pledge error handling polish #954

Merged
merged 5 commits into from
Nov 20, 2019

Conversation

Scollaco
Copy link
Contributor

@Scollaco Scollaco commented Nov 18, 2019

📲 What

  • Updates the error message when user tries to back a project with an amount greater than the maximum allowed.
  • Changes label's textAlignment to .right

🤔 Why

🛠 How

  • Replacing string
  • Changing textAlignment of label to .right
  • Updating tests and snapshots

👀 See

Before 🐛 After 🦋
Simulator Screen Shot - iPhone Xʀ - 2019-11-18 at 16 44 27 Simulator Screen Shot - iPhone Xʀ - 2019-11-18 at 15 39 54

♿️ Accessibility

  • Works with VoiceOver
  • Supports Dynamic Type

✅ Acceptance criteria

  • Select a reward from any project and type an amount greater than the maximum value. A label with the text The maximum pledge is [currency] [maximum pledge amount].
  • The error message should appear below the textField and aligned to the right.

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.

Have a small suggestion. Please let me know what you think! Awesome work on this 😄

@@ -158,11 +158,10 @@ public final class PledgeAmountViewModel: PledgeAmountViewModelType,
.map { ($0.0, $0.1) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just map to the max amount because we're not using the minimum amount anymore.

@Scollaco Scollaco merged commit 7ece968 into master Nov 20, 2019
@Scollaco Scollaco deleted the NT-526-max-pledge-error-handling-polish branch November 20, 2019 17:50
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.

2 participants