-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[Android] "What’s New" promo message: Implement pixels #7292
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
base: feature/ana/android_whats_new_promo_message_add_the_new_ui_elements
Are you sure you want to change the base?
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| companion object { | ||
| private const val PARAM_NAME_MESSAGE_ID = "message" | ||
| private const val PARAM_NAME_CARD_ID = "card" | ||
| internal const val PARAM_NAME_DISMISS_TYPE = "dismissType" |
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.
Bug: Pixel parameter name mismatch with JSON5 definition
The constant PARAM_NAME_DISMISS_TYPE is defined as "dismissType" (camelCase), but the pixel definition in remote_messaging_framework.json5 specifies the parameter key as "dismiss_type" (snake_case). This mismatch will cause the dismiss type parameter to not be properly recognized by the analytics system, resulting in anonymous measurement data being lost or incorrectly categorized.
Please tell me if this was useful or not with a 👍 or 👎.
| binding.headerImage.setImageResource(it.placeholder.drawable(true)) | ||
| binding.headerTitle.text = it.titleText | ||
| binding.actionButton.text = it.primaryActionText | ||
| viewModel.onMessageShown() |
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.
Bug: Missing duplicate prevention for "shown" pixels
The onMessageShown() call in render() lacks the duplicate prevention check present in the existing RemoteMessageView. The ViewModel computes newMessage (line 70) but doesn't include it in ViewState or use it to guard pixel firing. In contrast, the existing RemoteMessageView uses shouldRender = newMessage || binding.root.visibility == View.GONE before calling onMessageShown(). Without this check, the card "shown" pixels will fire every time render() is called with the same message, such as after configuration changes, leading to duplicate anonymous measurement data.
Please tell me if this was useful or not with a 👍 or 👎.
Additional Locations (1)
ff64a45 to
1957d95
Compare
73d3d60 to
579c0e6
Compare
…issals and update pixel tracking.
f56fe00 to
a2f7db4
Compare
579c0e6 to
9a50672
Compare
| val customParams = mapOf( | ||
| PARAM_NAME_DISMISS_TYPE to PARAM_VALUE_CLOSE_BUTTON, | ||
| ) | ||
| cardsListPixelHelper.dismissCardsListMessage(message.id, customParams) |
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.
Bug: Dismiss command sent before pixel and repository operations
In onCloseButtonClicked(), the Command.DismissMessage is sent before cardsListPixelHelper.dismissCardsListMessage() is called. This could cause a race condition where the activity finishes before the pixel fires and the message is dismissed in the repository. Compare with onBackPressed() in ModalSurfaceViewModel which correctly calls dismissCardsListMessage() first and sends the dismiss command afterward. The inconsistent ordering in onCloseButtonClicked() could result in missed anonymous measurements and messages not being properly marked as dismissed.
Please tell me if this was useful or not with a 👍 or 👎.

Task/Issue URL: https://app.asana.com/1/137249556945/project/1200581511062568/task/1211783737656685?focus=true
Description
Added pixel tracking for CardsList remote messages. This PR introduces a new
CardsListRemoteMessagePixelHelperclass that handles firing pixels for card-specific events (shown, clicked, dismissed) with appropriate parameters. The implementation tracks when cards are shown to users, when users interact with cards, and how messages are dismissed.Steps to test this PR
CardsList Remote Message Tracking
m_remote_message_card_shownpixel fires for each cardm_remote_message_card_clickedpixel fires with the correct card IDm_remote_message_dismissedpixel fires with the appropriate dismiss type parameterNO UI changes
Note
Add pixel tracking for CardsList remote messages (card shown/clicked, dismiss with type) and integrate firing on show, click, and back/close flows with tests.
m_remote_message_card_shown,m_remote_message_card_clicked, anddismiss_typeform_remote_message_dismissedinPixelDefinitions/pixels/remote_messaging_framework.json5.CardsListRemoteMessagePixelHelperwithRealCardsListRemoteMessagePixelHelperto firecard_shown,card_clicked, anddismissed(with custom params) and dismiss viaRemoteMessagingRepository.CardsListRemoteMessageViewModel: track currentRemoteMessage; on render callonMessageShown()to firem_remote_message_shownviaRemoteMessageModeland per-cardm_remote_message_card_shown; firem_remote_message_card_clickedon item click; on close, firem_remote_message_dismissedwithdismiss_type="close_button".ModalSurfaceViewModel+ModalSurfaceActivity: handle back navigation to firem_remote_message_dismissedwithdismiss_type="back_button_or_gesture"and dismiss.CardsListRemoteMessageViewcallsviewModel.onMessageShown()when content is rendered.dismiss_type.Written by Cursor Bugbot for commit 9a50672. This will update automatically on new commits. Configure here.