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

User can view shipping countries from pledge screen #517

Merged
merged 87 commits into from Jun 4, 2019
Merged

Conversation

Rcureton
Copy link
Contributor

@Rcureton Rcureton commented Apr 25, 2019

What ❓

  • This was a great collaboration between the three of us! @Scollaco @eoji πŸŽ‰
  • You can now view shipping countries from the pledge screen for rewards with shipping rules.
  • This is part of the native checkout feature.

How to QA? πŸ€”

  • Go to a project with the native checkout feature flag enabled -> select a reward -> on the pledge screen
  • If a reward has shipping rules then you will see the EditText which you can start typing the country you want to ship to.
  • If a reward has no shipping rules then the entire row will be gone.

Story πŸ“–

User can view shipping countries from pledge screen

See πŸ‘€

With Shipping Rules Without Shipping Rules

Apr-25-2019 16-48-06

eoji and others added 30 commits April 15, 2019 14:50
Added API call.
Added CenterSpan to center shrunken currency.
Added ShippingRule model.
Style updates.
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.

Took another look; I'm still experiencing the keyboard bug.

device-2019-05-28-173852 2019-05-28 17_39_30

android:layout_width="@dimen/plus_sign_width"
android:layout_height="@dimen/plus_sign_height"
android:backgroundTint="@color/ksr_grey_500"
android:src="@drawable/ic_add"
Copy link
Contributor

Choose a reason for hiding this comment

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

It's too small and it's the wrong color.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

A drawable has intrinsic padding so when you said a fixed size and not a scaletype, it doesn't take that into account. It's too small.

Copy link
Contributor

Choose a reason for hiding this comment

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

The positioning is also off.

@eoji
Copy link
Contributor

eoji commented May 31, 2019

I took another look and I can still reproduce both bugs.

  1. The keyboard bug is still happening and the 500 millisecond delay is very noticeable. The delay also happens even if the keyboard is not up so it just makes the app seem slow.
    device-2019-05-31-142450 2019-05-31 14_31_09

  2. The plus sign is still off. You can notice it especially when both are visible on the screen.

I would very much like to work on the checkout mutation this sprint and this PR is blocking that work. I think we should timebox these bugs to Tuesday EOD so this PR doesn't stay open any longer.

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.

I noticed a bug where when the user is refreshed, the shipping rule is changed but we can address that in a separate PR. Just have to merge some conflicts πŸ‘

# Conflicts:
#	app/src/main/java/com/kickstarter/libs/KSCurrency.java
#	app/src/main/java/com/kickstarter/ui/fragments/PledgeFragment.kt
#	app/src/main/java/com/kickstarter/viewmodels/HorizontalRewardViewHolderViewModel.kt
#	app/src/main/java/com/kickstarter/viewmodels/PledgeFragmentViewModel.kt
#	app/src/main/java/com/kickstarter/viewmodels/RewardViewModel.java
#	app/src/test/java/com/kickstarter/KSCurrencyTest.java
@Scollaco Scollaco merged commit ef33512 into master Jun 4, 2019
@Scollaco Scollaco deleted the reward-shipping branch June 4, 2019 21:35
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.

None yet

3 participants