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

MBL-1475 Add backing details navigation #2064

Merged
merged 10 commits into from
Jul 1, 2024
Merged

Conversation

ycheng-kickstarter
Copy link
Contributor

@ycheng-kickstarter ycheng-kickstarter commented Jun 26, 2024

📲 What

When tapping the project title and thumbnail area of the card, app should open a webview that loads the backing details url

🤔 Why

🛠 How

PPO cards are currently not hooked up to a backend query, so instead I added a backingDetailsUrl field to the PPOCardDataMock data object for now with a test url. Obviously once we hook up real PPO cards this test url should be replaced with the backend-provided url for the backing details.

👀 See

Hamburger menu -> Alerts -> tap on pledge summary view:
2662122e-016c-4181-9aca-ea81c1f2dc58

screen-20240626-163921.mp4

📋 QA

To get PPO cards to show up, on line 15 of the VM, replace
private val ppoCards = MutableStateFlow<PagingData<PPOCardDataMock>>(PagingData.empty())
with mock data:
private val ppoCards = MutableStateFlow<PagingData<PPOCardDataMock>>( PagingData.from( listOf( PPOCardDataMock(shippingAddress = "Firsty Lasty\n123 First St Apt #5678\nLos Angeles, CA 90025-1234\nUnited States", viewType = PPOCardViewType.CONFIRM_ADDRESS) ) ) )

Tapping the pledge summary view will open up the test url https://www.kickstarter.com/projects/thehoneycouple/the-honey-couples-building-expansion

Story 📖

https://kickstarter.atlassian.net/browse/MBL-1475

@@ -91,6 +91,7 @@
<!-- PPO Mock Strings -->
<string name="project_alerts_fpo">Project Alerts</string>
<string name="alerts_fpo">Alerts (%1$s)</string>
<string name="address_confirmed_snackbar_text">Address confirmed! Need to change your address before it locks? Visit your backing details on our website.</string>
<string name="address_confirmed_snackbar_text_fpo">Address confirmed! Need to change your address before it locks? Visit your backing details on our website.</string>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I snuck in this tiny string name change unrelated to this ticket because I realized I forgot to add "fpo" to the end of it

@@ -72,6 +74,8 @@ fun PledgedProjectsOverviewScreen(
errorSnackBarHostState: SnackbarHostState,
ppoCards: LazyPagingItems<PPOCardDataMock>,
totalAlerts: Int = 0,
onCardClick: () -> Unit,
onProjectPledgeSummaryClick: (backingDetailsUrl: String) -> Unit,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a new onProjectPledgeSummaryClick lambda for just the pledge summary tap area per the ticket AC. The onCardClick lambda is for the tap area of the entire card, not sure if it's going to end up being used?

Copy link
Contributor

Choose a reason for hiding this comment

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

The onCardClick i believe was meant to be used for this part of the feature, but it's fine. i'll just remove it at some point in a subsequent PR

@ycheng-kickstarter ycheng-kickstarter marked this pull request as ready for review June 27, 2024 16:35
Copy link
Contributor

@mtgriego mtgriego left a comment

Choose a reason for hiding this comment

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

Overall pretty good, though we should not be using RX2 in new viewmodels unless necessary for backwards compatibility etc

@@ -129,6 +133,7 @@ fun PledgedProjectsOverviewScreen(
PPOCardView(
viewType = it.viewType,
onCardClick = { },
onProjectPledgeSummaryClick = { onProjectPledgeSummaryClick(it.backingDetailsUrl) },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@leighdouglas Here's where we pass the backingDetailsUrl from the PPO card when user clicks on the tap area.

@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 85.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 68.07%. Comparing base (2ddec7c) to head (fbebe9d).

Files Patch % Lines
.../kickstarter/viewmodels/BackingDetailsViewModel.kt 84.21% 2 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2064      +/-   ##
============================================
+ Coverage     68.06%   68.07%   +0.01%     
- Complexity     2113     2114       +1     
============================================
  Files           355      356       +1     
  Lines         21652    21671      +19     
  Branches       3050     3051       +1     
============================================
+ Hits          14737    14753      +16     
- Misses         5273     5275       +2     
- Partials       1642     1643       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


lifecycleScope.launch {
repeatOnLifecycle(Lifecycle.State.STARTED) {
viewModel.url.collect { url ->
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

leighdouglas
leighdouglas previously approved these changes Jun 27, 2024
Copy link
Contributor

@leighdouglas leighdouglas left a comment

Choose a reason for hiding this comment

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

lgtm! 🎉

@ycheng-kickstarter ycheng-kickstarter self-assigned this Jun 27, 2024
Copy link
Contributor

@mtgriego mtgriego left a comment

Choose a reason for hiding this comment

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

LIttle more RX2 to remove

@mtgriego mtgriego merged commit 85a4c33 into master Jul 1, 2024
3 checks passed
@mtgriego mtgriego deleted the MBL-1475-backing-details-nav branch July 1, 2024 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants