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

Fix budget slider responsiveness #10739

Merged
merged 4 commits into from
Feb 12, 2024

Conversation

JorgeMucientes
Copy link
Contributor

@JorgeMucientes JorgeMucientes commented Feb 9, 2024

Part of: #10574

Description

These small changes fix several issues with the budget slider responsiveness. Issues are described in this PR: #10709 (comment)

Testing instructions

  • Open Blaze campaign creation flow
  • Open edit budget screen
  • Play sliding back and forth with the budget slider. Ensure that the total budget amount is responsive, and it always displays a value that is multiple of the campaign's duration value.

Images/gif

alwaysShowMultipleOfDuration.mp4

@@ -192,7 +195,6 @@ class BlazeCampaignBudgetViewModel @Inject constructor(
data class BudgetUiState(
val currencyCode: String,
val totalBudget: Float,
val sliderValue: Float,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The new approach is simpler and doesn't require to have 2 different values, one for the budget and other for the slider position

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Feb 9, 2024

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
FlavorJalapeno
Build TypeDebug
Commit9c9c872
Direct Downloadwoocommerce-prototype-build-pr10739-9c9c872.apk

@JorgeMucientes JorgeMucientes added the feature: blaze Related to the Blaze project label Feb 9, 2024
@JorgeMucientes JorgeMucientes marked this pull request as ready for review February 9, 2024 12:55
@dangermattic
Copy link
Collaborator

dangermattic commented Feb 9, 2024

1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@JorgeMucientes JorgeMucientes added the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Feb 9, 2024
@0nko 0nko self-assigned this Feb 9, 2024
Copy link
Contributor

@0nko 0nko left a comment

Choose a reason for hiding this comment

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

The fix works great. Nice work! 🏅

Base automatically changed from issue/10574-campaign-forecast to trunk February 12, 2024 11:00
@JorgeMucientes JorgeMucientes removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Feb 12, 2024
@peril-woocommerce
Copy link

Warnings
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

# Conflicts:
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/blaze/creation/budget/BlazeCampaignBudgetViewModel.kt
@JorgeMucientes JorgeMucientes merged commit 254d7ac into trunk Feb 12, 2024
14 of 15 checks passed
@JorgeMucientes JorgeMucientes deleted the issue/10574-fix-budget-slider-responsiveness branch February 12, 2024 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: blaze Related to the Blaze project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants