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

Migrate Horizontal rewards into the Project Page #504

Merged
merged 50 commits into from
Apr 11, 2019
Merged

Conversation

Rcureton
Copy link
Contributor

What ❓

  • Migrated the great work @eoji did with the Checkout POC into the KSR codebase for the first step towards Horizontal Rewards. On internal builds user's have the ability to toggle on/off the horizontal rewards from the InternalToolsActivity.
  • Great collaboration with @Scollaco 🎉🎉
  • We created the RewardFragment, RewardFragmentViewModel, HorizontalRewardViewHolder, HorizontalRewardAdapter,RewardFragmentViewModelTest. Also we updated the tests for ProjectViewModelTest

How to QA? 🤔

  • When you launch the app with an internal build open the navigation drawer -> InternalTools -> FeatureFlagActivity -> Toggle on or off.
  • When enabled navigate to the project page and you will no longer see the list of rewards below the project. When disabled you will see the project page in it's original state.

Story 📖

Migrate Horizontal Rewards

See 👀

Feature flag disabled Feature flag enabled

@Scollaco
Copy link
Contributor

@eoji
Copy link
Contributor

eoji commented Mar 25, 2019

Great job, y'all! Can't wait to dig into this 🕵️‍♀️

Copy link
Contributor

@eoji eoji left a comment

Choose a reason for hiding this comment

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

Great job! I did a first pass. I noticed we're missing a lot of this. but didn't want to have too many comments. Let's add those in.

app/src/main/res/layout/project_layout.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/project_layout.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/project_layout.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/project_layout.xml Show resolved Hide resolved
Copy link
Contributor

@eoji eoji left a comment

Choose a reason for hiding this comment

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

Did another pass on the updates! I'm glad we moved to just having the fragment live in the activity but rotations still aren't handled correctly. If the rewards are visible and you rotate, they should still be visible (but the animation should not repeat).

Copy link
Contributor

@eoji eoji left a comment

Choose a reason for hiding this comment

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

Alright, I think the major parts are good to go so here's some clean up comments

app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@eoji eoji left a comment

Choose a reason for hiding this comment

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

there's some feedback that still has not been addressed

app/src/main/res/layout/item_no_reward.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/item_reward.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/item_reward.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/project_layout.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@eoji eoji left a comment

Choose a reason for hiding this comment

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

Alright, did another sweep 🐣 I found some strange rotation behavior:

  1. In landscape, idk what the view is but there's something behind the card sheet so the curved card corners don't show.
  2. The position of the reward i'm viewing is not maintained. If I scroll to the last reward and rotate, I see the first reward.

Scollaco and others added 7 commits April 11, 2019 10:36
# Conflicts:
#	app/src/main/java/com/kickstarter/ui/activities/ProjectActivity.kt
#	app/src/main/java/com/kickstarter/viewmodels/ProjectViewModel.kt
#	app/src/main/res/layout/project_layout.xml
#	app/src/main/res/values/dimens.xml
#	app/src/test/java/com/kickstarter/viewmodels/ProjectViewModelTest.kt
@Rcureton Rcureton modified the milestone: Native Checkout Apr 11, 2019
Copy link
Contributor

@eoji eoji left a comment

Choose a reason for hiding this comment

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

WE DID IT
٩( ᐛ )و

@Rcureton Rcureton merged commit 6a903ac into master Apr 11, 2019
@Rcureton Rcureton deleted the horizontal-rewards branch April 11, 2019 21:09
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