Implemented the logic to hide the toolbar on scroll

[neeldoshii]

Fixes

• Fixes #14991

Approach

When the user scrolls down the toolbar will get hide and if he scrolls up the toolbar will get visible again. So, I have implemented a setOnScrollChangeListener which will keep a track on scroll behavior on the WebView performed by the user. So here if the user will scroll upwards so the toolbar will get hidden but if he scrolls downwards the toolbar will get visible again.

Testing Instructions

1. Open the Hamburger Menu
2. Open the Statistic tab
3. Perform Vertical Scroll and check the toolbar visiblity.

How Has This Been Tested?

This has been tested on physical device(Realme 3i) and on emulator (Google Pixel 3)

Here is the working video.

https://github.com/ankidroid/Anki-Android/assets/60827173/201c6d90-3750-41c1-8470-20587462f11b

Checklist

Please, go through these checks before submitting the PR.

• [x] You have a descriptive commit message with a short title (first line, max 50 chars).
• [x] You have commented your code, particularly in hard-to-understand areas
• [x] You have performed a self-review of your own code
• [ ] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• [ ] UI Changes: You have tested your change using the Google Accessibility Scanner

[welcome[bot]]

First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible.

[criticalAY]

If you noticed the toolbar flickers when we are scrolling up or down

[neeldoshii]

If you noticed the toolbar flickers when we are scrolling up or down.

Yes, its flickering a lot, I am making one more commit as a fix of flickering.

you can remove these comments.

If you don't mind what's the point in removing comments from the code? I have the habit of commenting for increased readability for new contributors/beginners to understands the codebase.

[neeldoshii]

@criticalAY I have fixed the issue of flickering on toolbar.

I have also removed the comments which was said earlier.

[criticalAY]

Code Quality

> Task :AnkiDroid:ktlintMainSourceSetCheck FAILED
/home/runner/work/Anki-Android/Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/pages/PageFragment.kt:65:1 Needless blank line(s)
/home/runner/work/Anki-Android/Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/pages/PageFragment.kt:68:1 Needless blank line(s)
/home/runner/work/Anki-Android/Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/pages/PageFragment.kt:77:61 Missing spacing before "{"

[neeldoshii]

Fix the lint error

Sure.

[neeldoshii]

@criticalAY lint error fixed

[neeldoshii]

The WebView object itself, as any other View, is able to use postDelayed, which should be enough

@Mukuljangir372 @BrayanDSO

I have changed the handler and used WebView object.

[neeldoshii]

The changes are being done in PageFragment, which means that all of its inheritors will get this behavior, but this was asked only for the Statistics screen

Can we have a toolbar hidden on scroll for every page? I guess it will be good from UX perspective? What do you think of it?

[neeldoshii]

@Mukuljangir372 I have added the 300 seconds in the const. and instead of toolbar?.visibility = View.GONE, I have used toolbar?.isVisible = false as requested.

Also, all the testcases have now been passed.

[BrayanDSO]

The hiding process is a bit rough IMO. Could you add some kind of animation? Like the one at the Settings screen when the toolbar is collapsed

[github-actions[bot]]

Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

[neeldoshii]

The hiding process is a bit rough IMO. Could you add some kind of animation?

Makes sense. Can we add fade in transition or something like that?

[BrayanDSO]

The hiding process is a bit rough IMO. Could you add some kind of animation?

Makes sense. Can we add fade in transition or something like that?

Sounds nice

[BrayanDSO]

It looks quite buggy here

Screen_recording_20240204_113037.webm

[user1823]

This comment is probably not helpful at all; I just wanted to share my thoughts.

Hiding the top region during scroll is very common in Android apps. I am quite surprised that implementing it is so difficult that the PR is pending since more than a month.

Though I am not a dev, for something this common, I would expect an easy solution (like Android allowing an app to say that this is the element that I want to hide on scroll — this would be a single-line code in the app and Android would handle the rest).

[mikehardy]

@user1823 just with regard to duration of PR working through progress - was also holidays and the team has been focused on higher priority 2.17 items, everything else is starved for attention

[user1823]

Just to clarify, I am not at all disappointed with the pace of development. You all are doing great work. I am just curious that why such a small thing is taking so much effort to develop.

By the way, with a quick Google search, I encountered the following page. It seems to be similar to what I envisioned and might be helpful. https://guides.codepath.com/android/handling-scrolls-with-coordinatorlayout

[neeldoshii]

@user1823

Hiding the top region during scroll is very common in Android apps. I am quite surprised that implementing it is so difficult that the PR is pending since more than a month.

Well, I don't mean to say anything but realistically I was quite busy as I was at the end of my internship so had heavy workload due to which I had no free time to contribute. Every contributor here & maintainer here are doing voluntary work each having different goal in mind to learn, upskill, get new feature of their favourite app, passion etc. We are currently focusing on releasing 2.17 so all other issues are done on lesser priority compared to higher priority issues tackled for 2.17 to roll up the update.

Coming towards technical part. The fix could have been done using a simple fix using coordinator layout but the issue arrived during that time was we had web view and web view scroll was not detected by coordinator layout due to which toolbar hide was not correctly. I added a programmatic manual approach in which handling animation and toggling scroll is hectic. I don't want to create excuses. I am sorry I could have tackled it using the hack of nested scroll and did it easily.

By the way, with a quick Google search, I encountered the following page. It seems to be similar to what I envisioned and might be helpful. https://guides.codepath.com/android/handling-scrolls-with-coordinatorlayout

Thank you though. I have fixed it, releasing a commit.

Thank you for your time.

[neeldoshii]

It looks quite buggy here

Screen_recording_20240204_113037.webm

https://github.com/ankidroid/Anki-Android/assets/60827173/0aef5d00-000a-45b8-8e37-1e842a9cc6d3

Did a slow, fast scroll, normal scroll. Works fine now.

Also one concerned in this current commit. We are using web view from page_fragment.xml. Adding coordinator layout in this will gonna make all the inherited class of pageFragment use this. Should we have separate xml for statistics?

[BrayanDSO]

Adding coordinator layout in this will gonna make all the inherited class of pageFragment use this. Should we have separate xml for statistics?

yes, that's the open/closed principle. We don't want to create any bugs because of it (I could find one in the Image Occlusion page).

Also, here's a patch on top of the current changes that fixes the status bar color (you still need to migrate this into a statistics xml):

Index: AnkiDroid/src/main/res/layout/page_fragment.xml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/res/layout/page_fragment.xml b/AnkiDroid/src/main/res/layout/page_fragment.xml
--- a/AnkiDroid/src/main/res/layout/page_fragment.xml	(revision 592b61462f4208198b11a8150082b75f7ec12659)
+++ b/AnkiDroid/src/main/res/layout/page_fragment.xml	(date 1707732870687)
@@ -3,12 +3,14 @@
     xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:app="http://schemas.android.com/apk/res-auto"
     android:layout_width="match_parent"
-    android:layout_height="match_parent">
+    android:layout_height="match_parent"
+    android:fitsSystemWindows="true">
 
     <com.google.android.material.appbar.AppBarLayout
         android:id="@+id/app_bar"
         android:layout_width="match_parent"
-        android:layout_height="wrap_content">
+        android:layout_height="wrap_content"
+        android:fitsSystemWindows="true">
 
     <com.google.android.material.appbar.MaterialToolbar
         android:id="@+id/toolbar"
@@ -23,14 +25,14 @@
     <androidx.core.widget.NestedScrollView
         android:layout_width="match_parent"
         android:layout_height="match_parent"
-        android:fillViewport="false"
-        android:fitsSystemWindows="true"
         app:layout_behavior="@string/appbar_scrolling_view_behavior">
-    <WebView
-        android:id="@+id/webview"
-        android:layout_width="match_parent"
-        android:layout_height="match_parent"
-        android:visibility="invisible"/>
+
+        <WebView
+            android:id="@+id/webview"
+            android:layout_width="match_parent"
+            android:layout_height="match_parent"
+            android:visibility="invisible"/>
+
     </androidx.core.widget.NestedScrollView>
 
 </androidx.coordinatorlayout.widget.CoordinatorLayout>
\ No newline at end of file

[github-actions[bot]]

Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

[neeldoshii]

Keep Open. Working!!!

[github-actions[bot]]

Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

[neeldoshii]

~Will take it once I get free, but for next 1-2 week I am on break so if someone wants to work on this issue feel free to work on this. Refer the above patch provided by Brayan, just need a refactor to get this working for just this implemented only in statistics screen.~

Edit : Released the fixed commit.