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-539] add background to reward section #950

Merged
merged 18 commits into from
Nov 19, 2019
Merged

Conversation

cdolm92
Copy link
Contributor

@cdolm92 cdolm92 commented Nov 14, 2019

📲 What

The white background is shown behind the reward section of the View/Manage Pledge screen for both backers and creators.

🛠 How

Reused rewardCardContainerView in ManagePledgeRewardView

👀 See

Check out snapshots too!

Before 🐛 After 🦋
Simulator Screen Shot - iPhone Xs - 2019-11-14 at 17 48 22 Simulator Screen Shot - Clone 1 of iPhone Xs - 2019-11-14 at 17 45 04

✅ Acceptance criteria

  • Navigate to the pledge view of a backed/live project your reward details should have a white background color.

@cdolm92 cdolm92 added the WIP label Nov 14, 2019
@cdolm92 cdolm92 added needs review and removed WIP labels Nov 15, 2019
Copy link
Contributor

@Scollaco Scollaco left a comment

Choose a reason for hiding this comment

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

I was taking a look at the code and actually we could make this work a bit simpler.
I opened this PR some time ago and what we can do is revert the changes (related to backgroundColor and styling) of RewardCardView and RewardCardViewModel and add a container UIViewon ManagePledgeRewardView with rounded corners and white background to place the reward view in it.

Doing this we will also avoid working with the complexity of the other RewardCardContainerView. I'm happy to pair on this if you want :)

Copy link
Contributor

@Scollaco Scollaco left a comment

Choose a reason for hiding this comment

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

Just a few more things, but it's looking good otherwise!

_ = self.rewardView
|> checkoutBackgroundStyle
_ = self.backgroundView
|> checkoutWhiteBackgroundStyle
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we can create a let backgroundViewStyle and move the styling to the bottom of the file.

@@ -12,6 +12,7 @@ final class ManagePledgeRewardView: UIView {

private lazy var rootStackView: UIStackView = { UIStackView(frame: .zero) }()
private lazy var titleLabel: UILabel = { UILabel(frame: .zero) }()
private lazy var backgroundView: UIView = { UIView(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.

Can you alphabetize here? 😬

Copy link
Contributor

@Scollaco Scollaco left a comment

Choose a reason for hiding this comment

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

Thank you for addressing the feedback, @cdolm92 . It looks great! Well done 🌟

@cdolm92 cdolm92 merged commit 45fbecd into master Nov 19, 2019
@cdolm92 cdolm92 deleted the background-to-reward-section branch November 19, 2019 16:30
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