[🆕] Manage pledge menu #588

eoji · 2019-08-14T22:16:15Z

📲 What

Showing an interstitial screen with menu options when the pledge sheet is expanded and the user has backed the project.

🤔 Why

MANAGE UR PLEDGE

🛠 How

ProjectActivity displays BackingFragment as the primary screen when the pledge sheet is opened.
- When it's visible, an options menu is present in the rewards toolbar.
- The menu has options for Update pledge, Change payment method, View rewards, Cancel pledge and Contact creator
Added PledgeReason that will be used to configure PledgeFragment UI
PledgeData.screenLocation is now optional.
PledgeFragment hides mini reward if no ScreenLocation.
Renamed RewardFragment to RewardsFragment.
Renamed RewardFragmentViewModel to RewardsFragmentViewModel.
Testz

👀 See

Menu❓	Update pledge	Change payment method

View rewards	Cancel Pledge	Contact creator

📋 QA

1️⃣When a user has backed a live project, clicking Manage reveals the Manage Your Pledge screen (it's blank for now) with an options menu.
2️⃣ The menu contains Update pledge, Change payment method, View rewards, Cancel pledge and Contact creator.
3️⃣ Update pledge -> Pledge screen (not currently configured)
4️⃣ Change payment method -> Pledge screen (not currently configured)
5️⃣ View rewards -> Rewards screen (scrolled to current reward)
6️⃣ Cancel pledge -> Cancel modal (needs new UI)
7️⃣ Contact creator -> Conversation with creator

Story 📖

https://dripsprint.atlassian.net/browse/NT-108

… the pledge sheet is opened. - When it's visible, an options menu is present in the rewards toolbar. - The menu has options for Update pledge, Change payment method, View rewards, Cancel pledge and Contact creator - Added PledgeReason that will be used to configure PledgeFragment UI - PledgeData.screenLocation is now optional. - PledgeFragment hides mini reward if no ScreenLocation. - Renamed RewardFragment to RewardsFragment. - Renamed RewardFragmentViewModel to RewardsFragmentViewModel. - Testz

roblum · 2019-08-15T03:28:32Z

app/src/main/java/com/kickstarter/ui/activities/ProjectActivity.kt

        }
    }

+    private fun renderProject(backingFragment: BackingFragment, rewardsFragment: RewardsFragment, project: Project) {


What happens when we have two methods with the same name? I see that we have another renderProject above with a different function signature. Is it able to detect which one to invoke based on the arguments?

Yep! I was considering naming it renderProjectInFragments but it's kinda long and redundant.

I guess I'm still learning how the files in this codebase are structured and what the purpose of each of these functions are. It looks like the one above renders an action button for the project? Could it be something like renderProjectActionButton or is that too specific? (disregard if I misunderstood)

ooh it's doing more than that. It's loading the project into the ProjectAdapter, loading the old style rewards if the native checkout flag is off, and configuring the button

roblum · 2019-08-15T03:51:17Z

app/src/test/java/com/kickstarter/viewmodels/ProjectViewModelTest.kt

+    }
+
+    @Test
+    fun testManagePledgeMenuIsVisible_whenProjectNotBacked() {


What do you think about adding Not to this test name testManagePledgeMenuIsNotVisible_whenProjectNotBacked

I've been trying to structure tests like test[output]_when[condition]. Does that make sense? Nino and I have been flopping around about it.

I think both make a lot of sense. Personally when I read tests that I'm unfamiliar with, I appreciate being able to get as much context from it without having to read through the whole test body. It saves me some time and allows me to jump back to the code.

Yeah, we've been discussing ways to make the tests more readable for people that are unfamiliar with the codebase. The pattern @eoji mentioned above is used on iOS too. Once you get into inputs/outputs it will make more sense, but maybe a discussion about it would be worth it.

Scollaco

Great work, @eoji . I left a some comments and questions.

app/src/main/java/com/kickstarter/ui/activities/ProjectActivity.kt

Scollaco · 2019-08-16T15:43:39Z

app/src/main/java/com/kickstarter/viewmodels/BackingFragmentViewModel.kt

+    interface Outputs {
+        /** Emits the current project. */
+        fun project(): Observable<Project>
+    }


Why is project being sent through input and being emitted as an output without any logic applied to it? Is this for tests purposes? 🤔

Yeah, just for tests. The designs for this screen aren't done so I just left it blank for now.

app/src/main/java/com/kickstarter/viewmodels/ProjectViewModel.kt

app/src/test/java/com/kickstarter/viewmodels/ProjectViewModelTest.kt

Scollaco · 2019-08-16T15:56:31Z

app/src/test/java/com/kickstarter/viewmodels/ProjectViewModelTest.kt

@@ -339,21 +351,21 @@ class ProjectViewModelTest : KSRobolectricTestCase() {
    }

    @Test
-    fun testHideRewardsFragment() {
+    fun testExpandPledgeSheet_whenHiding() {


Could this be renamed to testExpandPledgeSheet_whenHidingFragment just to be more explicit?

went with testExpandPledgeSheet_whenCollapsingSheet

Scollaco · 2019-08-16T15:56:43Z

app/src/test/java/com/kickstarter/viewmodels/ProjectViewModelTest.kt

    }

    @Test
-    fun testShowRewardsFragment() {
+    fun testExpandPledgeSheet_whenShowing() {


And here too

went with testExpandPledgeSheet_whenExpandingSheet

Scollaco · 2019-08-16T16:23:06Z

app/src/test/java/com/kickstarter/viewmodels/ProjectViewModelTest.kt

@@ -435,13 +447,18 @@ class ProjectViewModelTest : KSRobolectricTestCase() {
    }

    @Test
-    fun testScrimIsVisible() {
+    fun testScrimIsVisible_whenNativeCheckoutDisabled() {


Should we test the scrimIsVisible when native checkout is enabled as well?

yeah that's what happens in the next 2 tests

@Test fun testScrimIsVisible_whenNotBackedProject() { setUpEnvironment(environmentWithNativeCheckoutEnabled())

@Test fun testScrimIsVisible_whenBackedProject() { setUpEnvironment(environmentWithNativeCheckoutEnabled())

should I rename them?

Scollaco

Approved! (There's a alpha off due a renaming, but aside that it looks good) 😄

Scollaco · 2019-08-16T18:37:15Z

app/src/main/java/com/kickstarter/viewmodels/ProjectViewModel.kt

@@ -47,7 +53,7 @@ interface ProjectViewModel {
        fun heartButtonClicked()

        /** Call when horizontal rewards sheet should hide. */
-        fun hideRewardsSheetClicked()
+        fun collapsePledgeSheet()


And now the alpha is off lol

eoji added 3 commits August 13, 2019 14:04

wip backing fragment

a9c1390

wip showing and hiding rewards and backing fragments

cad2aad

roblum reviewed Aug 15, 2019

View reviewed changes

eoji added the feature ✨ label Aug 15, 2019

Scollaco reviewed Aug 16, 2019

View reviewed changes

Feedback from Nino

ed70142

Scollaco approved these changes Aug 16, 2019

View reviewed changes

the alphabet eludes me

de54975

eoji merged commit 72d9624 into master Aug 16, 2019

eoji deleted the manage-pledge-menu branch August 16, 2019 20:13

eoji mentioned this pull request Aug 28, 2019

[✨] View/Manage pledge UI #597

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[🆕] Manage pledge menu #588

[🆕] Manage pledge menu #588

eoji commented Aug 14, 2019

roblum Aug 15, 2019

eoji Aug 15, 2019

roblum Aug 15, 2019 •

edited

eoji Aug 15, 2019

roblum Aug 15, 2019

eoji Aug 15, 2019

roblum Aug 15, 2019

Scollaco Aug 16, 2019

Scollaco left a comment

Scollaco Aug 16, 2019

eoji Aug 16, 2019

Scollaco Aug 16, 2019

eoji Aug 16, 2019

Scollaco Aug 16, 2019

eoji Aug 16, 2019

Scollaco Aug 16, 2019

eoji Aug 16, 2019

Scollaco left a comment

Scollaco Aug 16, 2019

eoji Aug 16, 2019

[🆕] Manage pledge menu #588

[🆕] Manage pledge menu #588

Conversation

eoji commented Aug 14, 2019

📲 What

🤔 Why

🛠 How

👀 See

📋 QA

Story 📖

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roblum Aug 15, 2019 • edited

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

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

Choose a reason for hiding this comment

roblum Aug 15, 2019 •

edited