Add snippets for Jetpack PiP and notifications#900
Add snippets for Jetpack PiP and notifications#900alabiaga wants to merge 7 commits intoandroid:mainfrom
Conversation
Change-Id: Ia5887b6f73f0af4729544e1a30c82f915a58c054
|
Here is the summary of changes. You are about to add 3 region tags.
This comment is generated by snippet-bot.
|
There was a problem hiding this comment.
Code Review
This pull request introduces new code snippets for Android notifications and Picture-in-Picture (PiP) functionality, including the addition of the androidx.core.pip dependency. The feedback identifies several critical runtime issues, such as an invalid Intent constructor call and uninitialized lateinit properties in the new PiP activities. There are also suggestions to improve Kotlin idiomaticity by removing unnecessary semicolons and correcting descriptive comments.
Change-Id: I39e13958f811889e70b72c6e71256da7c3e75e72
Change-Id: I4661f0db980c7fea5131c95101f9cefe147fd8ad
| import android.content.Intent | ||
| import android.content.IntentFilter | ||
| import android.os.Build | ||
| import android.os.Bundle |
There was a problem hiding this comment.
Are these new imports in this file needed?
There was a problem hiding this comment.
nope, must've added this by mistake. removing
| implementation(libs.androidx.appcompat) | ||
| implementation(libs.androidx.core.ktx) | ||
| implementation(libs.androidx.core.pip) | ||
| implementation(libs.androidx.core.telecom) |
There was a problem hiding this comment.
is the telecom dependency needed?
There was a problem hiding this comment.
core.pip is but i don't know why telecom was pulled in. I'll remove
| } | ||
| } | ||
|
|
||
| override fun onDestroy() { |
There was a problem hiding this comment.
With Compose, instead of overriding activity lifecycle methods, I believe we prefer to use DisposableEffect inside a composable?
| private lateinit var pictureInPictureImpl: BasicPictureInPicture | ||
| override fun onCreate(savedInstanceState: Bundle?) { | ||
| super.onCreate(savedInstanceState) | ||
| pictureInPictureImpl = BasicPictureInPicture(this) |
There was a problem hiding this comment.
Do you need to clean up pictureInPictureImpl in this file too?
There was a problem hiding this comment.
i did clean it up in both files
There was a problem hiding this comment.
hm can you tell me which line number it is in this file? maybe I'm not seeing it..
There was a problem hiding this comment.
i instantiate pictureInPictureImpl in line 32 vs inline on line 29 as suggested by the assistant
Change-Id: I2f1a9b5c2366c6d7bb416e146bebaa40f78fe414
| private lateinit var pictureInPictureImpl: BasicPictureInPicture | ||
| override fun onCreate(savedInstanceState: Bundle?) { | ||
| super.onCreate(savedInstanceState) | ||
| pictureInPictureImpl = BasicPictureInPicture(this) |
There was a problem hiding this comment.
hm can you tell me which line number it is in this file? maybe I'm not seeing it..
| import androidx.compose.ui.platform.LocalContext | ||
| import androidx.core.app.PictureInPictureModeChangedInfo | ||
| import androidx.core.content.ContextCompat | ||
| import androidx.core.content.ContextCompat.getMainExecutor |
There was a problem hiding this comment.
is this import needed? I do see it used in the other files though
There was a problem hiding this comment.
removing..it's strange that these are getting added, as I am not touching this file at all.
Change-Id: I46f5785f220e884135395ae05246a2756de91848
| private lateinit var pictureInPictureImpl: BasicPictureInPicture | ||
| override fun onCreate(savedInstanceState: Bundle?) { | ||
| super.onCreate(savedInstanceState) | ||
| pictureInPictureImpl = BasicPictureInPicture(this) |
There was a problem hiding this comment.
i instantiate pictureInPictureImpl in line 32 vs inline on line 29 as suggested by the assistant
| import androidx.compose.ui.platform.LocalContext | ||
| import androidx.core.app.PictureInPictureModeChangedInfo | ||
| import androidx.core.content.ContextCompat | ||
| import androidx.core.content.ContextCompat.getMainExecutor |
There was a problem hiding this comment.
removing..it's strange that these are getting added, as I am not touching this file at all.
|
Checking in, let's discuss offline any open issues that aren't resolve and I'll follow up. |
Add code snippets for Jetpack PiP library
Start migrating embedded non snippet code samples for notification documentation in developer.android.com