[MBL-1815] PLOT plan selector - selected state #2213

jovaniks · 2024-12-05T17:43:11Z

📲 What

This PR implements the "Pledge Over Time" (PLOT) selected state in the Payment Plan Selector. It allows backers to choose the PLOT option and view the corresponding payment increments, including charge dates and amounts.

🤔 Why

The PLOT payment option enables backers to split their pledge amount into four equal payments, improving affordability and flexibility. This change is needed to meet the requirements for the PLOT initiative and ensure proper functionality within the payment plan selector.

🛠 How

Added a selectable "Pledge Over Time" (PLOT) option in the Payment Plan Selector.
Implemented logic to display mock payment increment records:
- Included charge dates (PledgePaymentIncrement).
- Displayed amounts for each payment (PledgePaymentIncrement.amount).
Payment increments are displayed in the their own currency.
Adapted the UI to follow the provided design:
- Added explanatory text below the PLOT option.
- Included Terms of Use link.
Mock data was used as the API is not yet ready.

👀 See

Jira
Figma

✅ Acceptance criteria

All tests pass
Confirm that payment increments (dates and amounts) are displayed correctly.
Verify the Terms of Use link functions as expected.
Verify the full description is displayed when selected.

⏰ TODO

Connect the component to the real API once it becomes available to replace mock data.

jovaniks · 2024-12-05T18:05:09Z

.../PledgePaymentPlansViewControllerTest/testView_PledgeOverTimeSelected.lang_en_device_pad.png

The spacing issue shown here is due to the test locking the layout to a specific screen size, which might not fully represent the component's dynamic behavior. In actual implementation, this component adapts its size based on the content and available screen dimensions, so the extra space seen in the test is expected.

Can you link the ticket you created here for reference? We should update how we use snapshot tests if we make a habit of testing individual views instead of the entire screen. (I'm a big fan of testing individual views, for the record; we just don't do that currently)

I haven’t created a ticket for this yet, but I can definitely do so to track this issue and the potential update to how we approach snapshot testing. Let me know if that works!

jovaniks · 2024-12-09T19:11:47Z

Library/ViewModels/PledgePaymentPlansViewModelTest.swift

+    self.vm.inputs.viewDidLoad()
+
+    self.vm.inputs.configure(with: self.selectionData)
+    self.vm.inputs.didTapTermsOfUse(with: .terms)


@scottkicks I checked the PLOT web version and they are showing the current Terms of use web page.

…alignment issue

jovaniks · 2024-12-11T00:43:59Z

Kickstarter-iOS/Features/PledgeOverTime/Views/PledgePaymentPlanOptionView.swift

+
+    if let firstDateLabel = dateLabels.first {
+      dateLabels.forEach { label in
+        label.widthAnchor.constraint(equalTo: firstDateLabel.widthAnchor).isActive = true


Intervals dates/amount alignment issue fixe

Explaining the issue: when the increments table mixed dates with one-digit days (4 Jan 2025) and two-digit days (14 Feb 2025), the amountLabel was misaligned. To fix it, a width constraint (all equal width) was added to each dateLabel.

scottkicks

Looking good! Just some clean up comments for you to consider.

scottkicks · 2024-12-11T16:15:46Z

.../PledgePaymentPlansViewControllerTest/testView_PledgeOverTimeSelected.lang_en_device_pad.png

It looks like there may be some extra spacing after "Charge 1". I know we were having issues without iPad snapshots for this previously. If it's the same issue here, that's fine for now. I'd like to fix that, though, if possible.

Of course, we need to address how we handle snapshot tests. I’ll create a ticket for this purpose. You can follow this thread for more details.

scottkicks · 2024-12-11T16:19:33Z

Kickstarter-iOS/Features/PledgeOverTime/Views/PledgePaymentPlanOptionView.swift

+        self.delegate?.pledgePaymentPlansViewController(self, didTapTermsOfUseWith: helpType)
+      }
+
+    self.termsOfUseButton.rac.hidden = self.viewModel.outputs.termsOfUseButtonHidden


Do we need this output? When the option isn't selected the entire PledgePaymentPlanOptionView will be hidden anyway, right? I don't think there's a case where the terms button won't be shown when everything else is.

Yes, we need it because the termsOfUseButton is part of the same stack view (optionDescriptorStackView) as the titleLabel, subtitleLabel, paymentIncrementsStackView, etc. Any recommendations or thoughts?

Ah I thought that there was a stackview just for the items that need to be hidden/shown at the correct time.

scottkicks · 2024-12-11T16:19:57Z

Kickstarter-iOS/Features/PledgeOverTime/Views/PledgePaymentPlanOptionView.swift

+      }
+
+    self.termsOfUseButton.rac.hidden = self.viewModel.outputs.termsOfUseButtonHidden
+    self.paymentIncrementsStackView.rac.hidden = self.viewModel.outputs.paymentIncrementsHidden


Actually same question about this stack view as the one above.

Same as with the termsOfUseButton, I could use the same output to manage all the controls that should be displayed when Pledge Over Time is selected. However, I prefer this solution as it provides control at the level of each individual UI element. Thoughts?

scottkicks · 2024-12-11T16:23:54Z

Kickstarter-iOS/Features/PledgeOverTime/Views/PledgePaymentPlanOptionView.swift

+      let titleLabel = UILabel()
+      applyIncrementTitleLabelStyle(titleLabel)
+      titleLabel.text = increment.title


nit: Can we rename this to something like chargeNumberLabel or something that makes it a bit clearer?

scottkicks · 2024-12-11T16:31:00Z

Kickstarter-iOS/Features/PledgeOverTime/Views/PledgePaymentPlanOptionView.swift

+private func applyTermsOfUseStyle(_ button: UIButton) {
+  button.configuration = {
+    var config = UIButton.Configuration.borderless()
+    config.contentInsets = NSDirectionalEdgeInsets(top: 1.0, leading: 0, bottom: 1.0, trailing: 0)


nit: we could throw these inset values, and any others if we have any, in a Constants enum. Similarly to EstimatedShippingCheckoutView.swift

Got it, I’ll move these contentInsets values (and any similar ones like spacing things) to a Constants enum for consistency. Thanks for the suggestion!

scottkicks · 2024-12-11T16:33:18Z

Library/ViewModels/NoShippingPledgeViewModel.swift

@@ -94,14 +94,18 @@ public class NoShippingPledgeViewModel: NoShippingPledgeViewModelType, NoShippin

    let backing = project.map { $0.personalization.backing }.skipNil()
    self.showPledgeOverTimeUI = project.signal
-      .map { ($0.isPledgeOverTimeAllowed ?? false) && featurePledgeOverTimeEnabled()
-      }
+      .map { _ in featurePledgeOverTimeEnabled() }


I think we can keep the project.isPledgeOverTimeAllowed check. Eventually, when this feature is fully rolled out and we clean up the feature flag we'll still only want to show PLOT UI if the project has isPledgeOverTimeAllowed set to true.

ouch my bad, I removed a bit for test, I'll rollback this validation

scottkicks · 2024-12-11T16:35:45Z

Library/ViewModels/NoShippingPledgeViewModel.swift


    self.pledgeOverTimeConfigData = self.showPledgeOverTimeUI
-      .map { showPledgeOverTimeUI -> PledgePaymentPlansAndSelectionData? in
+      .combineLatest(with: project)
+      .map { showPledgeOverTimeUI, project -> PledgePaymentPlansAndSelectionData? in
        guard showPledgeOverTimeUI else { return nil }


nit: this guard statement could be replaced with a .filter(showPledgeOverTimeUI). Then this output wouldn't need to return an optional PledgePaymentPlansAndSelectionData

scottkicks · 2024-12-11T16:36:59Z

Library/ViewModels/PledgePaymentPlansOptionViewModel.swift

 import ReactiveSwift

 public typealias PledgePaymentPlanOptionData = (
  type: PledgePaymentPlansType,
-  selectedType: PledgePaymentPlansType
+  selectedType: PledgePaymentPlansType,
+  paymentIncrements: [PledgePaymentIncrement], // TODO: replece with API model


Suggested change

paymentIncrements: [PledgePaymentIncrement], // TODO: replece with API model

paymentIncrements: [PledgePaymentIncrement], // TODO: replece with API model in [mbl-1838](https://kickstarter.atlassian.net/browse/MBL-1838)

Updated, thanks for adding the ticket ref

scottkicks · 2024-12-11T16:43:29Z

Library/ViewModels/PledgePaymentPlansOptionViewModel.swift

+  }
+}
+
+private func getPledgePaymentIncrementFormatted(


nit:

Suggested change

private func getPledgePaymentIncrementFormatted(

private func formattedPledgePaymentIncrement(

renamed, good suggestion

scottkicks · 2024-12-11T16:44:45Z

Library/ViewModels/PledgePaymentPlansViewModel.swift

  public var selectedPlan: PledgePaymentPlansType
+  public var paymentIncrements: [PledgePaymentIncrement]
+  public var project: Project
  /* TODO: add the necesary properties for the next states (PLOT Selected and Ineligible)
     - [MBL-1815](https://kickstarter.atlassian.net/browse/MBL-1815)


Are we be able to remove this comment now?

Sure, removed the line that is refer to the [MBL-1815]

nativeksr · 2024-12-11T18:50:50Z

	1 Warning
⚠️	Big PR

Generated by 🚫 Danger

…tate

scottkicks

👍 just one last minor suggestion from me

scottkicks · 2024-12-11T20:39:32Z

Library/ViewModels/NoShippingPledgeViewModel.swift


    self.pledgeOverTimeConfigData = self.showPledgeOverTimeUI
+      .filter { $0 }


nit: personally, I'd prefer to make this more explicit like .filter { showUI in showUI == true } but that's a minor nit.

jovaniks added 2 commits December 5, 2024 02:23

Add functionality for the Pledge Over Time option selected

7fda0e7

Fix Tests

5169766

jovaniks requested review from ifosli and amy-at-kickstarter December 5, 2024 17:43

jovaniks self-assigned this Dec 5, 2024

jovaniks commented Dec 5, 2024

View reviewed changes

jovaniks commented Dec 9, 2024

View reviewed changes

jovaniks requested a review from scottkicks December 9, 2024 20:57

Adding project to make the currency convertions and increments table …

ed401ec

…alignment issue

jovaniks commented Dec 11, 2024

View reviewed changes

jovaniks added 2 commits December 11, 2024 07:40

Test timestamp with constant

eff7fa4

Format

b5aab6c

scottkicks requested changes Dec 11, 2024

View reviewed changes

PR feedback

43bd0a5

jovaniks requested a review from scottkicks December 11, 2024 18:48

Merge branch 'main' into jluna/MBL-1815/plot-plan-selector-selected-s…

5db01cb

…tate

scottkicks approved these changes Dec 11, 2024

View reviewed changes

scottkicks mentioned this pull request Dec 11, 2024

[MBL-1816] PLOT Ineligible state #2220

Open

4 tasks

minor nit

2bcd664

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MBL-1815] PLOT plan selector - selected state #2213

[MBL-1815] PLOT plan selector - selected state #2213

jovaniks commented Dec 5, 2024 •

edited

Loading

jovaniks Dec 5, 2024

ifosli Dec 5, 2024

jovaniks Dec 5, 2024

jovaniks Dec 9, 2024

jovaniks Dec 11, 2024

scottkicks left a comment

scottkicks Dec 11, 2024

jovaniks Dec 11, 2024

scottkicks Dec 11, 2024

jovaniks Dec 11, 2024

scottkicks Dec 11, 2024

scottkicks Dec 11, 2024

jovaniks Dec 11, 2024

scottkicks Dec 11, 2024

scottkicks Dec 11, 2024

jovaniks Dec 11, 2024

scottkicks Dec 11, 2024

jovaniks Dec 11, 2024

scottkicks Dec 11, 2024

scottkicks Dec 11, 2024

jovaniks Dec 11, 2024

scottkicks Dec 11, 2024

jovaniks Dec 11, 2024

scottkicks Dec 11, 2024 •

edited

Loading

jovaniks Dec 11, 2024

nativeksr commented Dec 11, 2024

scottkicks left a comment

scottkicks Dec 11, 2024

	paymentIncrements: [PledgePaymentIncrement], // TODO: replece with API model
	paymentIncrements: [PledgePaymentIncrement], // TODO: replece with API model in [mbl-1838](https://kickstarter.atlassian.net/browse/MBL-1838)

	private func getPledgePaymentIncrementFormatted(
	private func formattedPledgePaymentIncrement(


		self.pledgeOverTimeConfigData = self.showPledgeOverTimeUI
		.filter { $0 }

[MBL-1815] PLOT plan selector - selected state #2213

Are you sure you want to change the base?

[MBL-1815] PLOT plan selector - selected state #2213

Conversation

jovaniks commented Dec 5, 2024 • edited Loading

📲 What

🤔 Why

🛠 How

👀 See

✅ Acceptance criteria

⏰ TODO

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

scottkicks 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

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

scottkicks Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nativeksr commented Dec 11, 2024

scottkicks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jovaniks commented Dec 5, 2024 •

edited

Loading

scottkicks Dec 11, 2024 •

edited

Loading