Skip to content
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

Fixed updating app bar color #6390

Merged
merged 9 commits into from
Sep 12, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,31 @@ package org.odk.collect.androidshared.ui
import android.content.Context
import android.os.Handler
import android.util.AttributeSet
import android.widget.ProgressBar
import com.google.android.material.progressindicator.LinearProgressIndicator

/**
* A progress bar that shows for a minimum amount fo time so it's obvious to the user that
* something has happened.
*/
class ObviousProgressBar(context: Context, attrs: AttributeSet?) : ProgressBar(context, attrs) {
class ObviousProgressBar(
context: Context,
attrs: AttributeSet?
) : LinearProgressIndicator(context, attrs) {
private val handler = Handler()
private var shownAt: Long? = null

fun show() {
init {
super.setVisibility(GONE)
super.setIndeterminate(true)
}

override fun show() {
handler.removeCallbacksAndMessages(null)
shownAt = System.currentTimeMillis()
super.setVisibility(VISIBLE)
}

fun hide() {
override fun hide() {
if (shownAt != null) {
val timeShown = System.currentTimeMillis() - shownAt!!

Expand Down
47 changes: 33 additions & 14 deletions androidshared/src/main/res/layout/app_bar_layout.xml
Original file line number Diff line number Diff line change
@@ -1,23 +1,42 @@
<?xml version="1.0" encoding="utf-8"?>
<!--
When scrolling, the top app bar fills with a contrasting color to create a visual separation.
This works automatically if your scrolling view (e.g., `RecyclerView`, `ListView`) is placed directly
beneath the `AppBarLayout`. However, if the scrolling view is nested within another view
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it "beneath" or is it if the two are children in the same CoordinatorLayout? It might be worth also linking to the docs here: https://developer.android.com/reference/com/google/android/material/appbar/AppBarLayout.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it "beneath" or is it if the two are children in the same CoordinatorLayout?

Both 😄 They are children in the same CoordinatorLayout and that layout works in the way that displayed the app bar above and the scrolling view beneath. If the scrolling view is there without any wrapper everything is fine out of the box.

It might be worth also linking to the docs here: https://developer.android.com/reference/com/google/android/material/appbar/AppBarLayout

This document does not emphasize the importance of the aspect mentioned in the comment at all, so I believe it is useless.

(such as a `ConstraintLayout`, which is common in this app), you need to help the app bar determine
whether it should lift by setting `app:liftOnScrollTargetViewId` to the ID of the scrolling view.
Since this `AppBarLayout` is used throughout the app with various scrolling views, it’s best to
use a shared ID like `scrollable_container`.
If the scrollable view is added programmatically or it is displayed in a `ViewPager` with a
shared id, it may not work as expected anyway, and `app:liftOnScrollTargetViewId` might
need to be updated programmatically after adding such a view.
The `ODKView` and its `odk_view_container` or `DeleteFormsActivity` are good examples of this scenario.
-->
<com.google.android.material.appbar.AppBarLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
xmlns:app="http://schemas.android.com/apk/res-auto"
android:id="@+id/appBarLayout"
android:layout_width="match_parent"
android:layout_height="wrap_content">
android:layout_height="wrap_content"
app:liftOnScrollTargetViewId="@+id/scrollable_container">

<com.google.android.material.appbar.MaterialToolbar
android:id="@+id/toolbar"
<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="?attr/actionBarSize"
tools:title="Title" />
android:layout_height="wrap_content">

<org.odk.collect.androidshared.ui.ObviousProgressBar
android:id="@+id/progressBar"
style="?android:attr/progressBarStyleHorizontal"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_below="@+id/appBarLayout"
android:layout_marginTop="-7dp"
android:indeterminate="true"
android:visibility="gone" />
<com.google.android.material.appbar.MaterialToolbar
android:id="@+id/toolbar"
android:layout_width="match_parent"
android:layout_height="?attr/actionBarSize"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
tools:title="Title" />

<org.odk.collect.androidshared.ui.ObviousProgressBar
android:id="@+id/progressBar"
android:layout_width="match_parent"
android:layout_height="wrap_content"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintBottom_toBottomOf="parent"/>
</androidx.constraintlayout.widget.ConstraintLayout>
</com.google.android.material.appbar.AppBarLayout>
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public void setContentView(@LayoutRes int layoutResID) {
}

private void init() {
listView = findViewById(android.R.id.list);
listView = findViewById(R.id.scrollable_container);
listView.setOnItemClickListener((AdapterView.OnItemClickListener) this);
listView.setEmptyView(findViewById(android.R.id.empty));
progressBar = findViewById(org.odk.collect.androidshared.R.id.progressBar);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import androidx.lifecycle.LifecycleOwner;
import androidx.lifecycle.ViewModelProvider;

import com.google.android.material.appbar.AppBarLayout;
import com.google.android.material.dialog.MaterialAlertDialogBuilder;

import org.javarosa.core.model.FormDef;
Expand Down Expand Up @@ -263,6 +264,7 @@ public class FormFillingActivity extends LocalizedActivity implements AnimationL
private Animation inAnimation;
private Animation outAnimation;

private AppBarLayout appBarLayout;
private FrameLayout questionHolder;
private SwipeHandler.View currentView;

Expand Down Expand Up @@ -468,6 +470,7 @@ public void onCreate(Bundle savedInstanceState) {

formError = null;

appBarLayout = findViewById(org.odk.collect.androidshared.R.id.appBarLayout);
questionHolder = findViewById(R.id.questionholder);

initToolbar();
Expand Down Expand Up @@ -1417,6 +1420,7 @@ public void showView(SwipeHandler.View next, FormAnimationType from) {
} else {
animationCompletionSet = 2;
}
appBarLayout.setLiftOnScrollTargetViewId(R.id.odk_view_container);
// start InAnimation for transition...
currentView.startAnimation(inAnimation);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public void onUploadButtonsClicked() {
public void setContentView(View view) {
super.setContentView(view);

listView = findViewById(android.R.id.list);
listView = findViewById(R.id.scrollable_container);
listView.setOnItemClickListener((AdapterView.OnItemClickListener) this);
listView.setEmptyView(findViewById(android.R.id.empty));
progressBar = findViewById(org.odk.collect.androidshared.R.id.progressBar);
Expand Down
2 changes: 1 addition & 1 deletion collect_app/src/main/res/layout/form_chooser_list.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<include layout="@layout/app_bar_layout"/>

<ListView
android:id="@id/android:list"
android:id="@+id/scrollable_container"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:nestedScrollingEnabled="true"
Expand Down
2 changes: 1 addition & 1 deletion collect_app/src/main/res/layout/form_download_list.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
app:layout_constraintTop_toTopOf="parent">

<ListView
android:id="@android:id/list"
android:id="@+id/scrollable_container"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:nestedScrollingEnabled="true"
Expand Down
2 changes: 1 addition & 1 deletion collect_app/src/main/res/layout/instance_uploader_list.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
app:layout_constraintEnd_toEndOf="parent"/>

<ListView
android:id="@android:id/list"
android:id="@+id/scrollable_container"
android:layout_width="match_parent"
android:layout_height="0dp"
android:nestedScrollingEnabled="true"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
app:layout_behavior="@string/appbar_scrolling_view_behavior">

<androidx.core.widget.NestedScrollView
android:id="@+id/scrollable_container"
android:layout_width="match_parent"
android:layout_height="0dp"
app:layout_constraintBottom_toBottomOf="@+id/shadow_up"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
app:layout_behavior="@string/appbar_scrolling_view_behavior">

<androidx.core.widget.NestedScrollView
android:id="@+id/scrollable_container"
android:layout_width="match_parent"
android:layout_height="0dp"
app:layout_constraintBottom_toBottomOf="@+id/shadow_up"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import android.view.ViewGroup
import androidx.core.view.isVisible
import androidx.fragment.app.Fragment
import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
import com.google.android.material.appbar.AppBarLayout
import org.odk.collect.androidshared.ui.FragmentFactoryBuilder
import org.odk.collect.lists.R
import org.odk.collect.lists.databinding.MultiSelectListBinding
Expand All @@ -19,6 +21,9 @@ class MultiSelectListFragment<T, VH : MultiSelectAdapter.ViewHolder<T>>(
private val onViewCreated: (MultiSelectListBinding) -> Unit = {}
) : Fragment() {

private var appBarLayout: AppBarLayout? = null
private lateinit var list: RecyclerView

override fun onAttach(context: Context) {
super.onAttach(context)

Expand Down Expand Up @@ -50,15 +55,19 @@ class MultiSelectListFragment<T, VH : MultiSelectAdapter.ViewHolder<T>>(
}

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
appBarLayout = requireActivity().findViewById(org.odk.collect.androidshared.R.id.appBarLayout)

val binding = MultiSelectListBinding.bind(view)
onViewCreated(binding)

binding.list.layoutManager = LinearLayoutManager(requireContext())
list = binding.list
list.layoutManager = LinearLayoutManager(requireContext())
val adapter = MultiSelectAdapter(
multiSelectViewModel,
viewHolderFactory
)
binding.list.adapter = adapter
list.adapter = adapter

multiSelectViewModel.getData().observe(viewLifecycleOwner) {
adapter.data = it
binding.empty.isVisible = it.isEmpty()
Expand All @@ -68,4 +77,9 @@ class MultiSelectListFragment<T, VH : MultiSelectAdapter.ViewHolder<T>>(
adapter.selected = it
}
}

override fun onResume() {
super.onResume()
appBarLayout?.setLiftOnScrollTargetView(list)
}
}