Fixed
Status Update
No update yet.
Comments
il...@google.com
Project: platform/frameworks/support
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
https://android-review.googlesource.com/1123258
https://goto.google.com/android-sha1/b90079595f33f58fece04026a97faa0d243acdb1
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
be...@upactivity.com
il...@google.com
Project: platform/frameworks/support
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
https://android-review.googlesource.com/1288456
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
be...@upactivity.com
> If you have a Toolbar in each Fragment, you should consider not using the ActionBar APIs at all
So basically if I have a toolbar in each fragment layout, and my activity has a theme without action bar:
1. I shouldn't use setupActionBarWithNavController
2. I have to set the "up" arrow icon myself when getting into the app hierarchy, inside the fragment code
3. I have to set all the titles myself, they're not derived from the navigation graph
Is that correct?
> Navigation drawers were always built to be a global mechanism for returning to a root set of destinations, so this seems to be WAI.
Not sure about that, see my Google+ example. The drawer is only visible on the main screens (stream, collections, communities, notifications), but it is not when getting deeper (post view, events, contacts, etc)
> > How to build a navigation graph with a drawer?
Actually my question here was more like "how to disable the drawer when getting deeper into the app".
Like how to disable the drawer when opening a post in Google+. Is this something that has to be done by hand, or is it handled by the navigation API?
> And you definitely can use <activity> destinations in your navigation graph
Thanks for the clarification about <activity> and more globally how to use the navigation graph.
So basically if I have a toolbar in each fragment layout, and my activity has a theme without action bar:
1. I shouldn't use setupActionBarWithNavController
2. I have to set the "up" arrow icon myself when getting into the app hierarchy, inside the fragment code
3. I have to set all the titles myself, they're not derived from the navigation graph
Is that correct?
> Navigation drawers were always built to be a global mechanism for returning to a root set of destinations, so this seems to be WAI.
Not sure about that, see my Google+ example. The drawer is only visible on the main screens (stream, collections, communities, notifications), but it is not when getting deeper (post view, events, contacts, etc)
> > How to build a navigation graph with a drawer?
Actually my question here was more like "how to disable the drawer when getting deeper into the app".
Like how to disable the drawer when opening a post in Google+. Is this something that has to be done by hand, or is it handled by the navigation API?
> And you definitely can use <activity> destinations in your navigation graph
Thanks for the clarification about <activity> and more globally how to use the navigation graph.
be...@upactivity.com
> It sounds like what you actually want is a setupToolbarWithNavController() equivalent that you could call in each Fragment?
After giving it more thought, and following your recommendations in the comments, actually maybe this is what I want/have to do.
So basically when setting up the fragment, I link to to the nav controller so that the navigation graph data is populated to the toolbar (home "up" button and title, basically).
The problem is, what is this `setupToolbarWithNavController()` you are referring to? It's not part of `NavigationUI`, is it?
I'm using the -alpha02 dependencies on 3.2 canary 17.
After giving it more thought, and following your recommendations in the comments, actually maybe this is what I want/have to do.
So basically when setting up the fragment, I link to to the nav controller so that the navigation graph data is populated to the toolbar (home "up" button and title, basically).
The problem is, what is this `setupToolbarWithNavController()` you are referring to? It's not part of `NavigationUI`, is it?
I'm using the -alpha02 dependencies on 3.2 canary 17.
il...@google.com
>So basically if I have a toolbar in each fragment layout, and my activity has a theme without action bar...
I think the proposed solution of having a setupToolbarWithNavController()-like method would do all the things you list, exactly like the setupActionBarWithNavController() does for the ActionBar.
> Not sure about that, see my Google+ example. The drawer is only visible on the main screens (stream, collections, communities, notifications), but it is not when getting deeper (post view, events, contacts, etc)
I'm not saying that Google+ does things correctly. I'm saying that the discussions with the material design team has made it clear that the navigation drawer is a global navigation pattern that should be available everywhere (in that respect, very similar to how the bottom nav bar should always be visible - also not something apps do consistently).
> Actually my question here was more like "how to disable the drawer when getting deeper into the app".
Like how to disable the drawer when opening a post in Google+. Is this something that has to be done by hand, or is it handled by the navigation API?
This isn't something Navigation would do automatically, but you can certainly add an OnNavigatedListener viahttps://developer.android.com/reference/androidx/navigation/NavController#addonnavigatedlistener and take any action you want, including locking the DrawerLayout closed.
I think the proposed solution of having a setupToolbarWithNavController()-like method would do all the things you list, exactly like the setupActionBarWithNavController() does for the ActionBar.
> Not sure about that, see my Google+ example. The drawer is only visible on the main screens (stream, collections, communities, notifications), but it is not when getting deeper (post view, events, contacts, etc)
I'm not saying that Google+ does things correctly. I'm saying that the discussions with the material design team has made it clear that the navigation drawer is a global navigation pattern that should be available everywhere (in that respect, very similar to how the bottom nav bar should always be visible - also not something apps do consistently).
> Actually my question here was more like "how to disable the drawer when getting deeper into the app".
Like how to disable the drawer when opening a post in Google+. Is this something that has to be done by hand, or is it handled by the navigation API?
This isn't something Navigation would do automatically, but you can certainly add an OnNavigatedListener via
il...@google.com
I'm proposing making this bug into a feature request for adding a setupToolbarWithNavController type of method if you're okay with that.
be...@upactivity.com
> I'm not saying that Google+ does things correctly. I'm saying that the discussions with the material design team has made it clear that the navigation drawer is a global navigation pattern that should be available everywhere (in that respect, very similar to how the bottom nav bar should always be visible - also not something apps do consistently).
Very interesting. Thanks for the insights.
> This isn't something Navigation would do automatically, but you can certainly add an OnNavigatedListener viahttps://developer.android.com/reference/androidx/navigation/NavController#addonnavigatedlistener and take any action you want, including locking the DrawerLayout closed.
Thanks again. That's what I would have done, I was just wondering if the navigation API had something automatic here. But if the drawer is a global navigation pattern and should always be available, that would be useless.
About these two items, it's weird because none of the apps I have tested quickly today seem to have a global drawer: Inbox, Gmail, Google+, Google Music, Play Store, Messages, Maps, Photos — all of them hide/lock the drawer when opening an item in the app (email, post, track, app, etc).
The only two notable exceptions are docs (spreadsheets etc): they open a new task when opening a document, so that makes sense; and YouTube, they got rid of the drawer altogether.
So that's a bit contradictory with a global navigation pattern, and it doesn't make me want to have a global drawer everywhere.
> I'm proposing making this bug into a feature request for adding a setupToolbarWithNavController type of method if you're okay with that.
That would be awesome. For an activity-managed ActionBar, use `NavigationUI.setupActionBarWithNavController()`; and for a custom Toolbar, use `NavigationUI.setupToolbarWithNavController()`. Both do the same: get information from the navigation graph (label, whether it's the start destination or not, menus, etc).
Is that right?
If so, please convert this to a feature request.
Thanks Ian! (I guess this is you :))
Very interesting. Thanks for the insights.
> This isn't something Navigation would do automatically, but you can certainly add an OnNavigatedListener via
Thanks again. That's what I would have done, I was just wondering if the navigation API had something automatic here. But if the drawer is a global navigation pattern and should always be available, that would be useless.
About these two items, it's weird because none of the apps I have tested quickly today seem to have a global drawer: Inbox, Gmail, Google+, Google Music, Play Store, Messages, Maps, Photos — all of them hide/lock the drawer when opening an item in the app (email, post, track, app, etc).
The only two notable exceptions are docs (spreadsheets etc): they open a new task when opening a document, so that makes sense; and YouTube, they got rid of the drawer altogether.
So that's a bit contradictory with a global navigation pattern, and it doesn't make me want to have a global drawer everywhere.
> I'm proposing making this bug into a feature request for adding a setupToolbarWithNavController type of method if you're okay with that.
That would be awesome. For an activity-managed ActionBar, use `NavigationUI.setupActionBarWithNavController()`; and for a custom Toolbar, use `NavigationUI.setupToolbarWithNavController()`. Both do the same: get information from the navigation graph (label, whether it's the start destination or not, menus, etc).
Is that right?
If so, please convert this to a feature request.
Thanks Ian! (I guess this is you :))
be...@upactivity.com
Please disregard my remark about the drawer and most apps, I guess I was decaffeinated because I was wrong for almost all of them.
il...@google.com
il...@google.com
A NavigationUI.setupWithNavController(Toolbar, NavController, DrawerLayout) method has been added internally and will be available in alpha03
Description
Version used:1.0.0-alpha02
Devices/Android versions reproduced on: Huawei P10+ Android 8.0
I have a single activity application with several fragments. They don't all have the same toolbar behavior/size/whatever so the main activity layout is a coordinator layout only:
<?xml version="1.0" encoding="utf-8"?>
<android.support.design.widget.CoordinatorLayout
android:id="@+id/main_activity_coordinator_layout"
xmlns:android="
xmlns:app="
xmlns:tools="
android:layout_width="match_parent"
android:layout_height="match_parent"
android:fitsSystemWindows="true"
tools:context=".app.MainActivity">
<fragment
android:id="@+id/main_activity_nav_host"
android:name="androidx.navigation.fragment.NavHostFragment"
android:layout_width="match_parent"
android:layout_height="match_parent"
app:defaultNavHost="true"
app:layout_behavior="@string/appbar_scrolling_view_behavior"
app:navGraph="@navigation/main_activity"/>
</android.support.design.widget.CoordinatorLayout>
Each fragment layout has an app bar layout and a toolbar.
In the main activity onCreate method I have the following:
NavigationUI.setupActionBarWithNavController(this, navController)
navController is a reference to the navigation controller of this activity.
This creates a NPE at ActionBarOnNavigatedListener.onNavigated(NavigationUI.java:284) because there is no action bar yet.
E Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'void android.support.v7.app.ActionBar.setTitle(java.lang.CharSequence)' on a null object reference
E at androidx.navigation.ui.NavigationUI$ActionBarOnNavigatedListener.onNavigated(NavigationUI.java:284)
E at androidx.navigation.NavController.addOnNavigatedListener(NavController.java:206)
E at androidx.navigation.ui.NavigationUI.setupActionBarWithNavController(NavigationUI.java:152)
E at androidx.navigation.ui.NavigationUI.setupActionBarWithNavController(NavigationUI.java:130)
E at xxx.app.MainActivity.onCreate(MainActivity.kt:161)
E at android.app.Activity.performCreate(Activity.java:7372)
E at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1218)
E at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3147)
E ... 9 more