-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Quick Search Notification Experiment #742
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
Merged
Merged
Changes from all commits
Commits
Show all changes
69 commits
Select commit
Hold shift + click to select a range
932903a
added notification query to BrowserActivity
malmstein 1904a97
Merge branch 'develop' into spike/david/sticky_search_notification
malmstein 674a502
making notification silent
malmstein 0dce32f
Merge branch 'develop' into spike/david/sticky_search_notification
malmstein 012df97
adding new prompt notification
malmstein 2f34416
Merge branch 'develop' into feature/david/sticky_search_notification
malmstein f7d490d
adding a new close button for all notification specs
malmstein 143e549
adding new search notification as silent
malmstein ad7f9a8
we want to update the notification instead of adding a new one
malmstein d9e192f
adding buttons to the search prompt
malmstein 83b434d
Merge branch 'develop' into feature/david/sticky_search_notification
malmstein b44cb8c
changing notification channel so it moves from the status bar to sile…
malmstein 1c45dde
adjusting notification background
malmstein 8d80ea9
added item to settings activity
malmstein 3596192
enabling / disabling the notification from settings screen
malmstein 08c22e2
no need to start the test notification here
malmstein d7881f9
moving to default priority, using the new interstitial search
malmstein 2bab692
notification should not appear in the lock screen
malmstein fd407bb
fixed failing tests
malmstein 3ef5f5e
search notification toggle should only be visible if experiment is en…
malmstein 9f1ef05
added tests for viewModel
malmstein 9437ca3
Merge branch 'develop' into feature/david/sticky_search_notification
malmstein 4520eb0
Merge branch 'develop' into feature/david/sticky_search_notification
malmstein f25176f
proper design for the search prompt notification
malmstein 82ad9e6
improved padding in the notification
malmstein b4da836
updated strings
malmstein e925b9e
remove Show from strings
malmstein 0b6f60a
updated action buttons text for notification
malmstein 6aac112
updated pixel names for the feature
malmstein 997257c
added new pixels and improved ui of initial notification
malmstein ed570f5
better padding
malmstein f391a41
updated background and padding for both notifications
malmstein fc510ec
using proper text title style for sticky notification
malmstein 6c9b20f
Merge branch 'develop' into feature/david/sticky_search_notification
malmstein 82a89c3
added missing tests for notification scheduler
malmstein 96b40ff
added comments on VariantManager
malmstein bc5e155
updated variants to latest definition
malmstein b9fcfdd
cleaning up the notification layout now that we don't need text
malmstein 508c74f
Revert "cleaning up the notification layout now that we don't need text"
malmstein add9906
cleaning up the layouts here
malmstein 61b226e
ensure we don't schedule or show the notification setting if the vari…
malmstein 8774c6c
proper notification variant keys
malmstein 0f7ac4e
good thing we had a test for this!
malmstein cd2344c
typo in test names
malmstein 522af65
no need to extend the open class when it's only done once
malmstein 0020de9
previous experiment already has it's sample size
malmstein a1ac5eb
xml formatting
malmstein 295538d
better naming for methods that create an Intent
malmstein 6e47ad9
merging issue, get rid of it
malmstein 81a384c
code formatting
malmstein 009de05
better method naming
malmstein 8b04005
these values are not used and should not be hardcoded
malmstein 751af30
ensure both active and inactive user notifications are scheduled as e…
malmstein 87431ca
clean up layout, we don't need the framelayout here
malmstein 9855ab0
remove unused imports
malmstein bcfa68d
extracting methods to its own constructor
malmstein 1b7c39c
this notification should use the same channel
malmstein c9a0aa6
changed notification channel to search an importance high so we alway…
malmstein cf078fa
added proper icons to add / remove
malmstein 43b66e3
fixing tests with proper values
malmstein a575823
we now send pixels to see engagement with notification from settings
malmstein 0df3fcb
cleaning up notification priorities
malmstein cb19bce
added empty drawable for better documentation
malmstein 7217661
better naming and proper empty drawable
malmstein 8ec320f
serp experiments are now inactive
malmstein d5df026
Merge branch 'develop' into feature/david/sticky_search_notification
malmstein 7c23dee
better name for the test
malmstein 49a5a1c
Merge branch 'develop' into feature/david/sticky_search_notification
malmstein 24d7b51
fixing merge issue
malmstein File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I like how we covered many test scenarios here 💯
I wonder if it's necessary to test what happens if we call twice
scheduleNextNotification, changing between calls which notification will appear.The motivation behind this is to test (if necessary) cancelation and scheduling a new notification having a previous one. Is that a possible case to cover?
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.
scheduleNextNotificationis something that happens once each time the app is opened, so it's the Application class responsibility to call this method. I don't think it's theScheduler'sresponsibility to test this scenario. Ideally I'd like to rethink how we schedule notifications, beingGlobalScope.launch()and all... but that's another story