-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Implements: Campaign listing page navigation and tracking #18833
Conversation
+ Adds: CampaignListingNavigation to the view model + Adds: navigation live data ↑ Updates: the logic to add navigation events
+ Adds: create campaign click on campaign listing ui state + Adds: compose view for create campaign + Adds: logic for navigating to blaze flow on create campaign fab click + Adds: content description for FAB button
📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
|
+ Adds: missing tests for CampaignListingViewModel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AjeshRPai - All is working as expected. The FAB looks like it belongs and matches.
I have one design concern fyi @osullivanchris - The tap area on the card title, which is what launches the campaigns list, is tiny. It may become larger once the more menu is added, but thought I'd mention it all the same.
.../src/test/java/org/wordpress/android/ui/blaze/blazecampaigns/CampaignListingViewModelTest.kt
Outdated
Show resolved
Hide resolved
Tapping a campaign brought me to an empty campaigns detail screen. Nothing ever loaded. Not sure if I am doing something wrong. I downloaded the APK above directly and used our shared test account. I didn't touch any feature flags.
Yep we should use the new FAB going forward everywhere. Looks and works as expected.
Yep seems fair. Let's assess once the more menu is in there. And the more menu should also have a link inside to all campaigns which gives us a fail safe. |
Hey @osullivanchris
At this time, the campaign detail page was not implemented hence it shows an empty page. If you want to test the updated app for campaigns feature, you can test using the build from this PR #18842 (review) |
Part of
#18800
#18533
#18528
Description
This PR implements the following in the Campaign listing page
Testing instruction
Click on FAB
Tracked: blaze_overlay_displayed, Properties: {"source":"campaign_listing_page”}
Campaign item click
Tracked: blaze_campaign_details_opened, Properties: {"source":"campaign_listing_page"}
No campaigns button click & FAB
Tracked: blaze_overlay_displayed, Properties: {"source":"campaign_listing_page"}
Note: The FAB implemented in this Page is a Jetpack compose replica of the current XML FAB implemented in the app with the PR - #18599, This FAB color is different to the one provided in the design. The one provided in the design was given before the FAB shape and color was changed across the Android PR, so I am guessing the one I have done is right. I am cc ing @osullivanchris so that he can review this when he comes back from AFK. Once he gives a design review of the FAB, I will take up the change if necessary. Lets go with the merge so that we can progress with the rest of the implementation
Regression Notes
Potential unintended areas of impact
Nothing as this is a new screen
What I did to test those areas of impact (or what existing automated tests I relied on)
N/A
What automated tests I added (or what prevented me from doing so)
N/A
PR submission checklist:
RELEASE-NOTES.txt
if necessary.UI Changes testing checklist: