Status Update
Comments
to...@gmail.com <to...@gmail.com> #2
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
lp...@google.com <lp...@google.com>
mo...@google.com <mo...@google.com> #3
to...@gmail.com <to...@gmail.com> #4
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
mo...@google.com <mo...@google.com> #5
I am seeing a problem with decorFitsSystemWindows = false
where it continues to invalidate the layout even after the animation completes. I don't think that's the problem you're worried about, though.
If I understand correctly, the problem you're seeing is something about it being laggy? And it didn't happen in the previous version, right?
That problem is likely due to the Window resizing on every frame, which is more expensive than the window content resizing on every frame. In a previous iteration, the window size was full screen and the content resized itself. You can resolve this yourself by doing this:
Surface(
modifier = Modifier.safeContentPadding()
.fillMaxSize()
.wrapContentHeight()
.fillMaxWidth(0.8f)
.animateContentSize(),
...
I'll see if I can fix the problem with decorFitsSystemWindows = false
invalidating layout continuously.
to...@gmail.com <to...@gmail.com> #6
There's 2 very different issue here. The main very blocking is the Samsung dialogs from a bottomsheet/popup being at the bottom and under the navigation bar. Samsung is a very large percent of users. And a full rewrite of all M3 AlerDialog to something else to workaround is a pain as most M3 things are private / internal.
The second issue is about the huge lag.
It was present since a very long time with usePlatformDefaultWidth = true
but not with usePlatformDefaultWidth = false
now it's always present.
AFAIR previously all the workaround based on .fillMaxSize()
had issues with the dialog scrim, but I'll try again if there's a fix for the Samsung devices.
mo...@google.com <mo...@google.com> #7
I just verified that an AlertDialog
called from a ModalBottomSheet
from a Pixel 9XL works fine. I'll see if I can find a Samsung device to test.
to...@gmail.com <to...@gmail.com> #8
Yes this is specific to Samsung as stated in first message.
I repro via Samsung testlab as I do not have a device myself, but A54 A55 with Android 14 there does reproduce the issue. (And a few different other Android 14 samsung devices from my beta users)
mo...@google.com <mo...@google.com> #9
I just verified that I can repro on a Samsung device.
to...@gmail.com <to...@gmail.com> #10
Perfect.
Just quickly tested the
.fillMaxSize()
.wrapContentHeight()
And it seems to behave correctly for the dialog scrim so works as a solution, may worth a note somewhere as cheating with usePlatformDefaultWidth = false
was often found as solution in a few posts over Internet.
A quick "mostly" related question to the dialog scrim, many users requested an option to hide the status bar in my app to be in full immersive mode, but there's no way to do that with dialogs right now. Is this something that can be requested? I have a fork of M3 BottomSheet to handle that, but handling a fork of the low level dialog requires a full copy and rewrite of everything that depends on it.
mo...@google.com <mo...@google.com> #11
Have you tried using the DialogWindowProvider?
:
fun findDialogWindowProviderInParent(view: View): DialogWindowProvider? {
if (view is DialogWindowProvider) {
return view
}
val parent = view.parent ?: return null
if (parent is View) {
return findDialogWindowProviderInParent(parent)
}
return null
}
@Composable
MyDialogContent() {
val dialogWindowProvider = findDialogWindowProviderInParent(LocalView.current)
dialogWindowProvider?.window?.addFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN)
...
}
to...@gmail.com <to...@gmail.com> #12
Not at all I always failed at finding the proper window, this works perfectly thanks a lot.
val dialogWindowProvider = findDialogWindowProviderInParent(LocalView.current)
val window = dialogWindowProvider?.window
if (window != null) {
WindowCompat.getInsetsController(window, window.decorView).apply {
hide(WindowInsetsCompat.Type.statusBars())
systemBarsBehavior = WindowInsetsControllerCompat.BEHAVIOR_SHOW_TRANSIENT_BARS_BY_SWIPE
}
}
to...@gmail.com <to...@gmail.com> #13
Returning to the 2nd issue about the lag and your solution.
Using a .fillMaxSize()
breaks the second .fillMaxWidth(0.8f)
. In my case since only the height change I can use a .fillMaxHeight()
but I do not really understand what is happening. See attached screenshots.
I can just return to usePlatformDefaultWidth = true
since your workaround fixes the lag and then it behave as expected.
So just reporting in case there's something else about measurement that changes and was not expected.
to...@gmail.com <to...@gmail.com> #14
Ok and now the memories slowly comes back :(
usePlatformDefaultWidth = true
and dialog resizing issues.
Use the following dialog content with the content from #4 and usePlatformDefaultWidth = true
Surface(
modifier = modifier
.safeContentPadding()
.fillMaxWidth(0.8f),
shape = shape,
color = backgroundColor,
contentColor = contentColor,
) {
dialogNavigator.SaveableState("currentDialog") {
dialogContent(dialogNavigator)
}
}
It gives the result plaformnoanimate.png attached.
With
Surface(
modifier = modifier
.safeContentPadding()
.animateContentSize(),
shape = shape,
color = backgroundColor,
contentColor = contentColor,
) {
dialogNavigator.SaveableState("currentDialog") {
dialogContent(dialogNavigator)
}
}
It gives the result platformanimate.png
Both are wrong as the dialog does not properly resized on content change, strangely I can't find the issue on the tracker but I'm pretty sure there was one opened about that since ages and the reasons we had to migrate to use usePlatformDefaultWidth = false
.
Those hacks are so old on my side that I have an hard time remembering all the details :(
.fillMaxSize()
breaks the dismissOnClickOutside
As per this item title, adding the fillMaxSize prevent click outside to dismiss the dialog so it not really a proper workaround.
All those tests are made with alpha 4 that contains the CL.
to...@gmail.com <to...@gmail.com> #15
Adding a final note if anyone reads and needs a solution.
Box(
modifier = Modifier
.safeDrawingPadding()
.fillMaxSize()
.padding(horizontal = 32.dp)
.clickable {
if (dialogState.canDismiss.value) {
dialogNavigator.hide()
}
}
) {
Surface(
modifier = modifier
.align(Alignment.Center)
.sizeIn(maxWidth = 640.dp)
.clickable(enabled = false) {}
.animateContentSize(),
shape = shape,
color = backgroundColor,
contentColor = contentColor,
) {
dialogNavigator.SaveableState("currentDialog") {
dialogContent(dialogNavigator)
}
}
}
Beware that this still only works properly with usePlatformDefaultWidth = false
See resulting video "works"
With usePlatformDefaultWidth = true
this will give the result "notwork" where the dialog width animate (while it should not, the actual width never changes) but the content does not seems to react so the title is on 2 lines. (The title is a simple Text() present on first display, not a later change)
I think I covered most of the issues with dialogs here. Some very old that where workarounded via usePlatformDefaultWidth=false
and some that are now also present in usePlatformDefaultWidth=false
due to window resizing (But working better than the other case)
ap...@google.com <ap...@google.com> #16
Project: platform/frameworks/support
Branch: androidx-main
Author: George Mount <
Link:
Set default Gravity for Dialogs to CENTER
Expand for full commit details
Set default Gravity for Dialogs to CENTER
Fixes: 373093006
On some devices, the default gravity for dialogs is not CENTER.
This changes the default gravity for Compose dialogs to CENTER.
Test: new test, manual testing on broken device
Change-Id: Ia2ec9f6edc2705cb8a13201d5e05a859a6bb9571
Files:
- M
compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/window/DialogTest.kt
- M
compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/window/AndroidDialog.android.kt
Hash: 97468d483b698d637162d50a4549ac0f2d8257ca
Date: Wed Oct 16 15:34:47 2024
to...@gmail.com <to...@gmail.com> #17
Can we discuss the other issues too?
to...@gmail.com <to...@gmail.com> #18
I guess that's a no and I documented all the issues for nothing.
pr...@google.com <pr...@google.com> #19
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.compose.ui:ui:1.8.0-alpha05
androidx.compose.ui:ui-android:1.8.0-alpha05
androidx.compose.ui:ui-jvmstubs:1.8.0-alpha05
androidx.compose.ui:ui-linuxx64stubs:1.8.0-alpha05
Description
Jetpack Compose version: 1.8 SNAPSHOT : 12425984
The CLhttps://android-review.googlesource.com/c/platform/frameworks/support/+/3272334 have broken dialogs shown from BottomSheets on Samsung Android 14 devices at least.
Snapshot 12425888 does not exhibit this behavior.
Repo: Show a Dialog/AlertDialog from a ModalBottomSheet from a recent M3 (So after the popup rewrite).
While this is probably a Samsung Bug (See the SAF confirmation screenshot that have the dialog at the bottom too), this is problematic as the M3 alert dialogs does not allow to pass a modifier to at least set contentSafePadding().
So if you consider that this is fully Samsung bug, it would be nice that this was reaffected to M3 to ensure the dialog can have safe paddings.