[NT-558][NT-508][NT-532] Indicate Backing state on Manage Pledge View #934

justinswart · 2019-11-07T01:47:16Z

📲 What

Adds text describing the Backing's state to ManagePledgeViewController.

🤔 Why

This provides some additional context to our backers that may be useful for them when viewing their pledge in the app.

🛠 How

Added strings for each state.
Added tests.
Repurposes attributedConfirmationString from PledgeViewModel to be a bit more reusable.
Updated snapshots for this view.
- Didn't record new snapshots for each state as I feel the view model tests are covering this.

Note: Once we are sure that we have the final copy for all of the strings I will add them for translation and rebuild the strings.

👀 See

Before 🐛	After 🦋

♿️ Accessibility

Works with VoiceOver
Supports Dynamic Type

🏎 Performance

Optimized Blended Layers (screenshots)

✅ Acceptance criteria

Note: It may be easiest to test by manipulating the JSON response using breakpoints with Charles proxy.

For both backer and creator:

Back a project, navigate to Manage Pledge. The copy that you see for each state should match the JIRA ticket.
Note that if a project has a canceled or failed state, we show different copy also noted on the ticket.

⏰ TODO

Add strings for translation.

ksr-ci-bot · 2019-11-07T01:59:32Z

	1 Warning
⚠️	Big PR

Generated by 🚫 Danger

justinswart · 2019-11-07T23:10:00Z

Apologies, this PR got a lot bigger than initially intended.

Scollaco

Paired with @justinswart on this PR and it looks good. I would just wait for the Strings to be deployed before merging it.

ifbarrera · 2019-11-11T15:42:44Z

Kickstarter-iOS/Views/Controllers/ManagePledgeSummaryViewController.swift

@@ -12,6 +12,10 @@ final class ManagePledgeSummaryViewController: UIViewController {
  private lazy var backerNumberLabel: UILabel = { UILabel(frame: .zero) }()
  private lazy var backingDateLabel: UILabel = { UILabel(frame: .zero) }()

+  private lazy var pledgeStatusLabelViewController: PledgeStatusLabelViewController = {


Just curious, why did you opt for using a VC here instead of a UIView? 🤔

I just had a good feeling about it 😄

ifbarrera · 2019-11-11T16:55:04Z

Library/ViewModels/PledgeStatusLabelViewModel.swift

+    NSAttributedString.Key.paragraphStyle: paragraphStyle
+  ]
+
+  if project.stats.currentCurrency == project.stats.currency {


You could use if project.stats.needsConversion here instead.

Or rather if !project.stats.needsConversion, though maybe it would make more sense to use a guard:

guard project.stats.needsConversion else { return Strings.If_the_project_reaches_its_funding_goal_you_will_be_charged_on_project_deadline( project_deadline: date ) .attributed(with: font, foregroundColor: foregroundColor, attributes: attributes, bolding: [date]) }

oh cool thanks!

ifbarrera · 2019-11-11T17:00:00Z

Library/ViewModels/PledgeStatusLabelViewModelTests.swift

+              |> Backing.lens.status .~ $0
+          )
+
+        self.vm.inputs.configure(with: project)


Do you need to call self.vm.inputs.viewDidLoad() somewhere?

ifbarrera · 2019-11-11T17:00:28Z

Library/ViewModels/PledgeViewModel.swift

-  attributedString.addAttributes(regularFontAttribute, range: fullRange)
-  attributedString.addAttributes(boldFontAttribute, range: rangePledgeTotal)
-  attributedString.addAttributes(boldFontAttribute, range: rangeProjectDeadline)
+  if project.stats.currentCurrency == project.stats.currency {


Here too we could use project.stats.needsConversion

This change seemed to break snapshots. Will dig in more tomorrow.

# Conflicts: # Kickstarter-iOS/Views/Controllers/ManagePledgeSummaryViewController.swift # Library/ViewModels/ManagePledgeSummaryViewModel.swift # Screenshots/_64/Kickstarter_Framework_iOSTests.ManagePledgeViewControllerTests/testView_GooglePay_lang_en_device_phone4_7inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ManagePledgeViewControllerTests/testView_NoReward_ApplePay_lang_en_device_phone4_7inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ManagePledgeViewControllerTests/testView_lang_de_device_pad@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ManagePledgeViewControllerTests/testView_lang_de_device_phone4_7inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ManagePledgeViewControllerTests/testView_lang_en_device_pad@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ManagePledgeViewControllerTests/testView_lang_en_device_phone4_7inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ManagePledgeViewControllerTests/testView_lang_es_device_pad@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ManagePledgeViewControllerTests/testView_lang_es_device_phone4_7inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ManagePledgeViewControllerTests/testView_lang_fr_device_pad@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ManagePledgeViewControllerTests/testView_lang_fr_device_phone4_7inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ManagePledgeViewControllerTests/testView_lang_ja_device_pad@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ManagePledgeViewControllerTests/testView_lang_ja_device_phone4_7inch@2x.png

Scollaco

I only left a couple of suggestions, but none are blockers.

Scollaco · 2019-11-12T20:49:28Z

Library/ViewModels/PledgeStatusLabelViewModel.swift

+    self.labelText = project.map(statusLabelText(with:)).skipNil()
+  }
+
+  private let configureWithProjectStatusProperty = MutableProperty<Project?>(nil)


Maybe configureWithProjectProperty here could be more appropriated, since we're getting not only the project's state, but the whole project?

justinswart added 3 commits November 6, 2019 17:29

Add strings for backing status on manage pledge view

211a3ab

Merge branch 'master' into manage-pledge-backing-state-string

2a8aed8

Add copy for error state

1b95b41

justinswart requested review from Scollaco and cdolm92 November 7, 2019 01:47

justinswart assigned cdolm92 Nov 7, 2019

justinswart added 2 commits November 6, 2019 17:50

Formatting

d2fe813

Update new snapshots

b1dd8da

justinswart added 3 commits November 7, 2019 14:47

Merge branch 'master' into manage-pledge-backing-state-string

705e402

Improved sharing of functions, added creator strings

aa6584c

Correct font

329baca

justinswart changed the title ~~[NT-508] Indicate Backing state on Manage Pledge View~~ [NT-508][NT-532] Indicate Backing state on Manage Pledge View Nov 7, 2019

justinswart added 3 commits November 7, 2019 15:23

Use smart apostrophe

780791f

Add attributed string tests

6bc8502

Remove creator strings

28a7e86

justinswart assigned Scollaco and unassigned cdolm92 Nov 8, 2019

Scollaco approved these changes Nov 8, 2019

View reviewed changes

Merge branch 'master' into manage-pledge-backing-state-string

83c5f3a

ifbarrera reviewed Nov 11, 2019

View reviewed changes

Use Strings functions ahead of translations

292a150

ifbarrera reviewed Nov 11, 2019

View reviewed changes

justinswart added 3 commits November 11, 2019 20:26

German, Spanish and French translations

a1a8dce

Use project.stats.needsConversion, fix test

a37fedc

Merge branch 'master' into manage-pledge-backing-state-string

4b1123b

justinswart mentioned this pull request Nov 12, 2019

[NT-534] Show User Name and Avatar on the Manage Pledge screen #938

Merged

5 tasks

Fix tests, record new snapshots

e42b807

justinswart requested a review from Scollaco November 12, 2019 16:37

justinswart added 7 commits November 12, 2019 11:37

Merge branch 'master' into manage-pledge-backing-state-string

1641f6c

Remove traitCollectionDidChange input

7a725e0

Merge branch 'master' into manage-pledge-backing-state-string

0a8d8c7

Formatting

c4cdf0a

New snapshots

c709f2c

Use a UIView instead

3cfabdb

Scollaco approved these changes Nov 12, 2019

View reviewed changes

Rename property

1d2d54b

justinswart changed the title ~~[NT-508][NT-532] Indicate Backing state on Manage Pledge View~~ [NT-558][NT-508][NT-532] Indicate Backing state on Manage Pledge View Nov 12, 2019

justinswart merged commit eccc8dc into master Nov 12, 2019

justinswart deleted the manage-pledge-backing-state-string branch November 12, 2019 21:45

This was referenced Nov 13, 2019

[NT-569] Fix PledgeAmountSummaryViewController configuration regression #945

Merged

[NT-532] Add backing status strings for creator context #951

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NT-558][NT-508][NT-532] Indicate Backing state on Manage Pledge View #934

[NT-558][NT-508][NT-532] Indicate Backing state on Manage Pledge View #934

justinswart commented Nov 7, 2019 •

edited

Loading

ksr-ci-bot commented Nov 7, 2019 •

edited

Loading

justinswart commented Nov 7, 2019

Scollaco left a comment

ifbarrera Nov 11, 2019

justinswart Nov 12, 2019

ifbarrera Nov 11, 2019

ifbarrera Nov 11, 2019

justinswart Nov 12, 2019

ifbarrera Nov 11, 2019

justinswart Nov 12, 2019

ifbarrera Nov 11, 2019

justinswart Nov 12, 2019

Scollaco left a comment

Scollaco Nov 12, 2019

[NT-558][NT-508][NT-532] Indicate Backing state on Manage Pledge View #934

[NT-558][NT-508][NT-532] Indicate Backing state on Manage Pledge View #934

Conversation

justinswart commented Nov 7, 2019 • edited Loading

📲 What

🤔 Why

🛠 How

👀 See

♿️ Accessibility

🏎 Performance

✅ Acceptance criteria

⏰ TODO

ksr-ci-bot commented Nov 7, 2019 • edited Loading

justinswart commented Nov 7, 2019

Scollaco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Scollaco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinswart commented Nov 7, 2019 •

edited

Loading

ksr-ci-bot commented Nov 7, 2019 •

edited

Loading