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-1340] Expandable Rewards Pledge View Header #1226

Merged
merged 5 commits into from
Jun 17, 2020

Conversation

justinswart
Copy link
Contributor

@justinswart justinswart commented Jun 16, 2020

📲 What

Adds the new expanded reward header to PledgeViewController.

Note: Subsequent PR's will add the expand/collapse functionality. Also, currently only the reward will be shown as there are no add-ons yet.

🤔 Why

A continuation of the work that we're doing for add-on rewards, this allows us to display a list of rewards and add-ons on the pledge view.

🛠 How

  • Added PledgeExpandableRewardsHeaderViewController and PledgeExpandableRewardsHeaderDataSource.
  • Added cells PledgeExpandableHeaderRewardHeaderCell and PledgeExpandableHeaderRewardCell.
  • Added VM tests, snapshot tests.

👀 See

Before 🐛 After 🦋
image image

♿️ Accessibility

  • Supports Dynamic Type

🏎 Performance

  • Optimized Blended Layers (screenshots)

✅ Acceptance criteria

  • Navigate to the pledge view, observe the new header is displayed for regular rewards (not No Reward rewards).
  • Navigate through the various manage contexts and observe that this header is only shown when in Update pledge, none of the other contexts.

@justinswart justinswart requested a review from jgsamudio June 16, 2020 21:24
@justinswart justinswart changed the title Nt 1340 expandable header [NT-1340] Expandable Rewards Pledge View Header Jun 16, 2020
@nativeksr
Copy link
Collaborator

1 Warning
⚠️ Big PR

Generated by 🚫 Danger


private enum Layout {
enum Margin {
static let width: CGFloat = Styles.grid(3)
Copy link
Contributor

Choose a reason for hiding this comment

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

In the future it might be worth standardizing the styles across the whole app.

final class PledgeExpandableHeaderRewardCell: UITableViewCell, ValueCell {
// MARK: - Properties

private lazy var amountLabel: UILabel = { UILabel(frame: .zero) }()
Copy link
Contributor

Choose a reason for hiding this comment

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

If its a single line, I think you can just do:

Suggested change
private lazy var amountLabel: UILabel = { UILabel(frame: .zero) }()
private lazy var amountLabel: UILabel = UILabel(frame: .zero)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah cool, we've always just written our lazy props this way but no reason to add closures if it's not needed! Will update here 👍

// MARK: - Properties

private lazy var amountLabel: UILabel = { UILabel(frame: .zero) }()
private lazy var rootStackView: UIStackView = { UIStackView(frame: .zero) }()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private lazy var rootStackView: UIStackView = { UIStackView(frame: .zero) }()
private lazy var rootStackView: UIStackView = UIStackView(frame: .zero)


private lazy var amountLabel: UILabel = { UILabel(frame: .zero) }()
private lazy var rootStackView: UIStackView = { UIStackView(frame: .zero) }()
private lazy var titleLabel: UILabel = { UILabel(frame: .zero) }()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private lazy var titleLabel: UILabel = { UILabel(frame: .zero) }()
private lazy var titleLabel: UILabel = UILabel(frame: .zero)

private func rootStackViewStyle(_ isAccessibilityCategory: Bool) -> (StackViewStyle) {
let alignment: UIStackView.Alignment = (isAccessibilityCategory ? .leading : .top)
let axis: NSLayoutConstraint.Axis = (isAccessibilityCategory ? .vertical : .horizontal)
let distribution: UIStackView.Distribution = (isAccessibilityCategory ? .equalSpacing : .fill)
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ for Accessibility!

final class PledgeExpandableHeaderRewardHeaderCell: UITableViewCell, ValueCell {
// MARK: - Properties

private lazy var amountLabel: UILabel = { UILabel(frame: .zero) }()
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to update here if you want to remove the brackets

@justinswart justinswart merged commit 9cb0fcc into NT-1140-add-ons-manage-context Jun 17, 2020
@justinswart justinswart deleted the NT-1340-expandable-header branch June 17, 2020 15:41
justinswart added a commit that referenced this pull request Sep 2, 2020
* [NT-1295] Add UITableView and data source to ManagePledgeViewController (#1213)

* [NT-636] "Fix" button on Manage Pledge screen for errored backings (#982)

* Fix button on errored pledges

* Tests

* Formatting

* Screenshot tests

* Rerecording tests

* formatting

* [NT-636] "Fix" button on Manage Pledge screen for errored backings (#982)

* Fix button on errored pledges

* Tests

* Formatting

* Screenshot tests

* Rerecording tests

* formatting

* [NT-635] Update "fix" state of CTA button (#993)

* Added missing strings

* [NT-634] Errored pledges top cell (#991)

* [NT-635] CTA fix button navigation (#1027)

* [NT-973] Fix payment icon error (#1104)

* fix image shown

* commit adds fixIconHidden output and tests this output. Also corrects some tests

* fix test

* snapshot

* renames erroredPledge to isErrored

* swift format

* fixes tests errors

* Revert "fixes tests errors"

This reverts commit 32cea0f.

* uses isErrored

* pr feedback

* [NT-974, NT-637] Navigate to fix pledge screen/ Fix Pledge state (#1109)

* fix image shown

* commit adds fixIconHidden output and tests this output. Also corrects some tests

* fix test

* snapshot

* renames erroredPledge to isErrored

* swift format

* fixes tests errors

* Revert "fixes tests errors"

This reverts commit 32cea0f.

* uses isErrored

* navigating to fix screen

* testing gotofixpledge output

* corrected merge and removed unused constant

* fix payment method copy

* testing pledgeContext

* seift format

* wip

* wip

* pr feedback

* swift format

* pr feedback

* swiftformat

* [NT-214] Fix pledge tracking events (#1128)

* wip- 2/3 events

* tests

* Should fix feature branch (#1141)

* should fix feature branch

* Update Library/ViewModels/PledgeViewModel.swift

pr feedback

Co-Authored-By: jgsamudio <jonathan2457@gmail.com>

* swift format

* Revert "swift format"

This reverts commit 89b6a0d.

* swift format

* Revert "swift format"

This reverts commit eb9261b.

* swift format

Co-authored-by: jgsamudio <jonathan2457@gmail.com>

* [NT-1072] Whitelist fix pledge tracking events for data lake (#1142)

* whitelisted tracking events

* swift format

* fix test

* pr feedback

* [NT-686] add pledge collection date query (#1140)

* wip - collection date feature complete

* tests passing

* swift format

* wip- refactor

* fix tests

* snapshots

* updated copy

* swift format

* pr feedback

* cache formatter

* swift format

* New snapshots

* [NT-1182] Fetch Backing for ManagePledgeViewController from GraphQL (#1174)

* Fetch Backing for ManagePledgeViewController from GraphQL

* Correct test names, use rawValue in GraphSchema

* Remove Argo import

* Put GraphBackingEnvelope back

* Fix snapshot tests

* Record snapshots from commandline

* [NT-1185] Configure CancePledgeViewController with GraphQL Backing (#1176)

* Configure CancePledge with GraphQL backing

* Update tests

* Fix merge error

* Record snapshots from commandline

* [NT-1183, NT-1184] Configure ManagePledgeViewController subviews with GraphQL Backing (#1188)

* Configure ManagePledgeSummaryView with GraphQL Backing

* Fix snapshot tests

* Fix tests

* Make reward optional

* Refactor, ensure all sub-vc's don't depend on Project directly

* Move test

* Put test back

* Remove coalesce

* Alphabetize

* Tuples -> Structs

* Convert to double during deserialization

* Fix tests

* Remove recordMode

* [NT-633] Errored Pledge Deeplink (#1178)

* Errored pledge deeplink

* RefTag tests

* Revert debug code

* Use correct navigation controller

* Present login modal when logged out during errored pledge deeplink

* Present login modal when logged out

* Update visibility modifiers

* Fix failing test

* [NT-650, NT-1135, NT-1052] Add Errored Pledges to badge count (#1193)

* Add errored pledges to badge count and refresh user on fix pledge

* Fix test, updating backings on user update

* Ensure errored backing are cleared, shipping amount is shown in summary

* Add test

* Add retry payment method behaviour

* ManagePledgeView loading state

* Don't un-hide rootStackView if backing fails to load

* Don't hide apple pay button in fix context

* Add pull-to-refresh on error

* [NT-1247] ManagePledgeViewController loading state (#1203)

* Add retry payment method behaviour

* ManagePledgeView loading state

* Don't un-hide rootStackView if backing fails to load

* Don't hide apple pay button in fix context

* Show apple pay button when changing payment method

* Test commit - will revert

* Revert "Test commit - will revert"

This reverts commit 5e196f0.

* Fix layout

* Fix merge error

* Fix project file

* Hide rightBarButtonItem when view is errored

* ignoreValues instead

* Refactor to instantiate ManagePledgeViewModel with params only

* Fix ManagePledgeViewBackingEnvelopeTests

* [NT-1274] Pull to refresh on failure to fetch Project/Backing (#1206)

* Add retry payment method behaviour

* ManagePledgeView loading state

* Don't un-hide rootStackView if backing fails to load

* Don't hide apple pay button in fix context

* Add pull-to-refresh on error

* Fix layout

* Fix merge error

* Hide rightBarButtonItem when view is errored

* ignoreValues instead

* Fix tests

* Fix tests

* Consolidate shared functions, prefer filterMap

* Refactor ManagePledgeViewController to use a table view

* Fix layout issues with card view

* Add data source test

* Fix presentation crash, only show payment method section if a credit card was deserialized

* Allow optional backing param

* Remove logEvents

* Remove logEvents

* [NT-1269, NT-1270] Deprecate BackingViewController (#1212)

* Add retry payment method behaviour

* ManagePledgeView loading state

* Don't un-hide rootStackView if backing fails to load

* Don't hide apple pay button in fix context

* Add pull-to-refresh on error

* Fix layout

* Fix merge error

* Hide rightBarButtonItem when view is errored

* ignoreValues instead

* Refactor to instantiate ManagePledgeViewModel with params only

* Fix ManagePledgeViewBackingEnvelopeTests

* Fix tests

* Fix tests

* Consolidate shared functions, prefer filterMap

* Fix presentation crash, only show payment method section if a credit card was deserialized

* Allow optional backing param

* Remove logEvents

* Remove unused code

* [NT-1223, NT-1296] Reward Add-Ons GraphQL integration (#1216)

* Map add-on data to Reward

* Add quantity to reward title

* Pass quantity through

* Add boolean for isAddOn

* Move ID

* Add tests

* Set timezone in unit test

* Add test for query

* [NT-1298] Manage Pledge Shipping Disclaimer View (#1217)

* Add disclaimer view to manage pledge view

* Remove text color from style

* Update snapshots

* Add snapshots

* Hide pledge disclaimer view when no shipping or user is creator

* Update snapshots

* Fix Library tests

* [NT-1297] Add bonus support (#1218)

* Updates to reward received vc

* Add tests

* Add context-specific string

* Add bonus amount

* Update query

* Fix query

* Fix query

* Configure RewardCardView with context, hide bonus on no reward, fix corner radius

* Fix tests

* Fix snapshot test

* Layout fix

Co-authored-by: Isabel Barrera <isabel@kickstarter.com>
Co-authored-by: Nino Collaco <scollaco@gmail.com>
Co-authored-by: Isabel Barrera <ifbarrera@me.com>
Co-authored-by: christella <cdolm92@gmail.com>
Co-authored-by: jgsamudio <jonathan2457@gmail.com>

* [NT-1340] Expandable Rewards Pledge View Header (#1226)

* Add expandable header to pledge view

* Fix tests

* Fix test

* Remove closures from lazy props

* Replace PledgeDescriptionView with project title label (#1229)

* [NT-1341 ] Rewards header expand/collapse animation (#1230)

* Add expand/collapse animation for rewards header

* New snapshots

* Silence autolayout warning

* Increase button tappable area

* Fix layout

* [NT-1329, NT-1330] Bonus Support UI (#1233)

* Add bonus support to pledge view

* Update snapshots

* Fix padding on pledge disclaimer view (#1235)

* [NT-1330] Fix bonus support behaviour and validation (#1236)

* Fix bonus support behaviour and validation

* Fix test

* Fix info displayed on manage pledge view (#1241)

* [NT-1405] Use bonus support as total for No Reward (#1240)

* Use bonus amount as total for no reward

* Fix snapshot test

* [NT-1140] Add-Ons Phase 2 (#1245)

* [NT-1366] Add-ons Selection Navigation (#1243)

* Navigate to add-ons selection

* Actually stage all the files

* Put import back

* [NT-1366] Add header UI to add-ons selection view (#1244)

* Add basic header view with label

* Blend background color

* [NT-1367] Add-Ons List (#1246)

* Add RewardAddOnSelectionViewController

* Remove logEvents

* Update comment

* [NT-1368] Reward Add-On Card UI (#1248)

* Reward Add-On Card UI

* Update Kickstarter-iOS/Views/PillsView.swift

Co-authored-by: jgsamudio <jonathan2457@gmail.com>

* Fix tests

Co-authored-by: jgsamudio <jonathan2457@gmail.com>

* Styling fixes (#1249)

* Add add button and stepper to reward add-on card (#1250)

* [NT-1370] Filter Add-Ons by Location (#1251)

* Filter add-ons by location

* Typo

* Typo

* Fix snapshot tests

* Update inclusion logic

* Update Library/ViewModels/RewardAddOnSelectionViewModel.swift

Co-authored-by: Bessie Chan <bessie@kickstarter.com>

* [NT-1371] Add-on Reward Selection and Continue Button (#1252)

* Add-on selection and CTA

* Remove unused code

Co-authored-by: Bessie Chan <bessie@kickstarter.com>

* [NT-1371, NT-1331, NT-1391, NT-1400, NT-1436] Create/Update Backing with Add-Ons (#1253)

* Filter add-ons by location

* Typo

* Typo

* Fix snapshot tests

* Update inclusion logic

* Add-on selection and CTA

* Remove unused code

* Update create/updating backing mutations

* Pledging for add-ons and regular update flows working, tests failing

* KsApi tests

* Create/UpdateBackingInput constructor tests

* Fixing tests

* Add on selection tests

* PledgeViewModel and CancelPledgeViewModel tests

* Fix Library tests

* Fix remaining failing tests

* Remove record statement

* Fix Apple Pay totals

* Remove log statement

* SanitizedParams tests

* Remove unused code, add PledgeShippingSummaryViewModelTests

* ManagePledgeViewModelTests

* More tests

* Fix padding, add snapshot test

* Fix amounts during manage flow

* Fix snapshots

* Fix add-on shipping calculation

* Fix snapshots

* [NT-1450] Allow Dissimilar Shipping Cost for Add-Ons (#1257)

* Display individual add-on's shipping rule

* Update comment

* Misc add-on card fixes (#1258)

* [NT-1401] Reward Card UI updates (#1259)

* Add you backed label and add-ons pill

* Restructure view to fix button position on scroll

* Change title to Edit Reward (#1260)

* Update bonus amount in USD to not be cents (#1262)

* [NT-1439] Add Empty States and Pull-to-Refresh (#1261)

* Add empty states and pull-to-refresh

* Remove recordMode

* [NT-1455, NT-1458] Add-Ons GraphQL Refactor (#1263)

* KsApi updates

* Add add-ons to Backing

* Hide Graph models from the main app

* Include projects in BackingsEnvelope

* Add finalCollectionDate

* More API fixes

* KsApi updates

* Fix Library framework tests

* Fix tests

* Add staticUsdRate

* Add test

* Fix backing being passed through to pledge view

* Remove extra header space

* Pass shipping preference through from GraphReward (#1264)

Co-authored-by: jgsamudio <jonathan2457@gmail.com>
Co-authored-by: Bessie Chan <bessie@kickstarter.com>

* Update max pledge string (#1266)

* Remove rounding on bonus support tracking property (#1267)

* [NT-1372, NT-1459] Update Backing with Add-ons (#1268)

* Fix memory leak on pledge view, pass quantities through edit reward flow

* Update snapshot tests

* [NT-1416] Edit Add-Ons Reward Alert (#1269)

* Display prompt when editing a backing with add-ons

* Improve readability

* Fix grammar in comment

* Refresh shipping locations on error (#1270)

* Fix tests (#1271)

* Prevent race condish (#1272)

* [NT-1481] Remove Amount Shipping Locations List (#1273)

* Remove amount from shipping location list

* Fix tests

* Update snapshots

* Tracking for add-ons (#1276)

* Add haptics to add-on selection card (#1277)

* Update translations (#1275)

* [NT-1416] Navigation bugfix (#1274)

* Rearrange signals

* Don't navigate forward if creator of project

* Update alert copy (#1278)

Co-authored-by: Isabel Barrera <isabel@kickstarter.com>
Co-authored-by: Nino Collaco <scollaco@gmail.com>
Co-authored-by: Isabel Barrera <ifbarrera@me.com>
Co-authored-by: christella <cdolm92@gmail.com>
Co-authored-by: jgsamudio <jonathan2457@gmail.com>
Co-authored-by: Bessie Chan <bessie@kickstarter.com>
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.

3 participants