Automatic Scroll on Search added #988

4shutosh · 2023-03-18T11:28:57Z

📷 Screenshots

📄 Context

Scroll to Search content added, fixes #388

📝 Changes

Scroll to search item in response screen added

📎 Related PR

🚫 Breaking

No

🛠️ How to test

Open Response Screen, ideally where the whole body covers more than the screen size, and search for something that is not visible in the current scroll state to see the change.

After searching if found, the recycler view is automatically scrolled to the position, making it easy to test/evaluate the response. Earlier, just the text was being highlighted making it hard to find in a lengthy response.

⏱️ Next steps

4shutosh · 2023-03-18T11:55:53Z

No visual change from the previous search results, different color can be added for the navigating item, but is that really required?
For a single search result, the up and down arrow buttons are not visible and are enabled/disabled on the ends of the list of search results (Gray color for that)

Any suggestions for code improvements, behavior change are highly appreciated.

4shutosh · 2023-03-20T17:55:44Z

Please take a look @vbuberen @cortinico

cortinico · 2023-04-10T16:35:47Z

Please take a look @vbuberen @cortinico

Thanks for sendign this over @4shutosh This week I'm quite busy with KotlinConf, I won't be able to review this before the end of the conference. I can check it just after 👍
I also know that @vbuberen was interested in implementing something like this in the past so maybe he has relevant insights on this PR

ArjanSM

Hi @4shutosh 👋
Looks good mostly!
I just have a few comments, looking forward to everyone's opinion.

...rc/main/kotlin/com/chuckerteam/chucker/internal/ui/transaction/TransactionPayloadFragment.kt

ArjanSM · 2023-04-10T19:03:54Z

library/src/main/res/layout/chucker_fragment_transaction_payload.xml

+    <com.google.android.material.floatingactionbutton.FloatingActionButton
+        android:id="@+id/fab_to_next"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:backgroundTint="@color/chucker_fab_background_colour"
+        android:contentDescription="@string/chucker_scroll_buttons_for_search"
+        android:src="@drawable/chucker_ic_arrow_down"
+        android:visibility="gone"
+        app:fabSize="mini"
+        app:layout_constraintBottom_toBottomOf="parent"
+        app:layout_constraintEnd_toEndOf="parent"
+        app:tint="@color/chucker_color_on_primary"
+        tools:visibility="visible" />
+
+    <com.google.android.material.floatingactionbutton.FloatingActionButton
+        android:id="@+id/fab_to_previous"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:layout_marginEnd="4dp"
+        android:backgroundTint="@color/chucker_fab_background_colour"
+        android:contentDescription="@string/chucker_scroll_buttons_for_search"
+        android:enabled="false"
+        android:rotation="180"
+        android:src="@drawable/chucker_ic_arrow_down"
+        android:visibility="gone"
+        app:fabSize="mini"
+        app:layout_constraintBottom_toBottomOf="parent"
+        app:layout_constraintEnd_toStartOf="@id/fab_to_next"
+        app:tint="@color/chucker_color_on_primary"
+        tools:enabled="false"
+        tools:visibility="visible" />
+


While the FABs do seem to provide an efficient usage of the real estate on the screen I feel displaying a summary of the filtered search results vis-a-vis the current highlighted search result would be more disambiguous.
For e.g <Searched Text> 3/11

We could chose to display the search results info summary by extending the App Bar to also include the icons(chevrons) which could be used to traverse through the filtered search results.
@cortinico @vbuberen thoughts?

We could chose to display the search results info summary by extending the App Bar to also include the icons(chevrons) which could be used to traverse through the filtered search results.

This would be preferred IMHO. Specifically having the # of occurrences would be helpful (see my video in top comment also).

Also I would refrain from using FloatingActionButton as they're specific to MDC and I'm looking into having MDC being used as a compileOnly dependency. So using classes directly inside the XML file makes things a bit harder.

Yeah, I agree.

Will do the changes to summarise the search results in the app bar, along with the navigation chevrons.

cortinico

Great work @4shutosh

I've been playing a bit with this feature and I believe, as @ArjanSM suggested, we need a bit more of a visual clue on the # of occurrences.

For example, look at this flow where all the occurrences are next to each other:

video-1682250533.mp4

Having no visual clue makes it impossible to know if there are "more" occurrences later. Also, I think we might highlight differently the selected occurrence (like with a different color).

CHANGELOG.md

cortinico · 2023-04-23T11:38:55Z

library/src/main/res/layout/chucker_fragment_transaction_payload.xml

+    <com.google.android.material.floatingactionbutton.FloatingActionButton
+        android:id="@+id/fab_to_next"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:backgroundTint="@color/chucker_fab_background_colour"
+        android:contentDescription="@string/chucker_scroll_buttons_for_search"
+        android:src="@drawable/chucker_ic_arrow_down"
+        android:visibility="gone"
+        app:fabSize="mini"
+        app:layout_constraintBottom_toBottomOf="parent"
+        app:layout_constraintEnd_toEndOf="parent"
+        app:tint="@color/chucker_color_on_primary"
+        tools:visibility="visible" />
+
+    <com.google.android.material.floatingactionbutton.FloatingActionButton
+        android:id="@+id/fab_to_previous"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:layout_marginEnd="4dp"
+        android:backgroundTint="@color/chucker_fab_background_colour"
+        android:contentDescription="@string/chucker_scroll_buttons_for_search"
+        android:enabled="false"
+        android:rotation="180"
+        android:src="@drawable/chucker_ic_arrow_down"
+        android:visibility="gone"
+        app:fabSize="mini"
+        app:layout_constraintBottom_toBottomOf="parent"
+        app:layout_constraintEnd_toStartOf="@id/fab_to_next"
+        app:tint="@color/chucker_color_on_primary"
+        tools:enabled="false"
+        tools:visibility="visible" />
+


We could chose to display the search results info summary by extending the App Bar to also include the icons(chevrons) which could be used to traverse through the filtered search results.

This would be preferred IMHO. Specifically having the # of occurrences would be helpful (see my video in top comment also).

Also I would refrain from using FloatingActionButton as they're specific to MDC and I'm looking into having MDC being used as a compileOnly dependency. So using classes directly inside the XML file makes things a bit harder.

...rc/main/kotlin/com/chuckerteam/chucker/internal/ui/transaction/TransactionPayloadFragment.kt

cortinico · 2023-04-23T11:44:47Z

...rc/main/kotlin/com/chuckerteam/chucker/internal/ui/transaction/TransactionPayloadFragment.kt

+            listOfScrollableIndex.isEmpty() -> {
+                // scroll to top
+                currentSearchScrollIndex = 0
+                payloadBinding.payloadRecyclerView.smoothScrollToPosition(0)


I'm unsure this is good from the UX point of view (i.e. the user is searching and you keep on scrolling up and down).

I prefer the implemented way, quickly jumping to the position. However, I can think of two solutions here.

We can increase the initial delay.

Or we can only scroll after the IME_ACTION_DONE or keyboard closed - along with the summary of search results in the app bar.

Sorry my comment was related to the re-scroll to top once listOfScrollableIndex.isEmpty

Oh okay, yeah let's not scroll in that condition. Will do the change.

cortinico · 2023-04-29T14:09:13Z

CHANGELOG.md

@@ -15,6 +14,7 @@ Please add your entries according to this format.
 * Added ability to export transactions to a file programmatically, LOG or HAR.
 * GraphQL OperationName header to transaction title [#69], [#116]
 * Added support for Android 13 and notifications permission handling
+* Added scroll to highlighted text search in response screen [#988]eye


Suggested change

* Added scroll to highlighted text search in response screen [#988]eye

* Added scroll to highlighted text search in response screen [#988]

cortinico · 2023-04-29T14:57:11Z

@4shutosh can you also resolve the conflicts?

4shutosh · 2023-04-29T14:57:39Z

yes, on it.

4shutosh · 2023-04-29T15:58:51Z

Added green color for the search item in focus, the search summary is in app bar now, and the nav chevron icons allow to loop through all the search results.

cortinico · 2023-04-29T19:15:48Z

This looks great! 👍
There are still a couple of issues that we need to fix before this can go in:

I was playing with it and I managed to let the app crash with a zero division.

Process: com.chuckerteam.chucker.sample, PID: 15747
                                                                                                    java.lang.ArithmeticException: divide by zero
                                                                                                    	at com.chuckerteam.chucker.internal.ui.transaction.TransactionPayloadFragment.onSearchScrollerButtonClick(TransactionPayloadFragment.kt:172)
                                                                                                    	at com.chuckerteam.chucker.internal.ui.transaction.TransactionPayloadFragment.onViewCreated$lambda$3(TransactionPayloadFragment.kt:155)
                                                                                                    	at com.chuckerteam.chucker.internal.ui.transaction.TransactionPayloadFragment.$r8$lambda$X8t2VmZq9zp3O9oB7hdlJ8Q2YEc(Unknown Source:0)
                                                                                                    	at com.chuckerteam.chucker.internal.ui.transaction.TransactionPayloadFragment$$ExternalSyntheticLambda2.onClick(Unknown Source:2)
                                                                                                    	at android.view.View.performClick(View.java:7455)
                                                                                                    	at android.view.View.performClickInternal(View.java:7432)
                                                                                                    	at android.view.View.access$3700(View.java:835)
                                                                                                    	at android.view.View$PerformClick.run(View.java:28810)
                                                                                                    	at android.os.Handler.handleCallback(Handler.java:938)
                                                                                                    	at android.os.Handler.dispatchMessage(Handler.java:99)
                                                                                                    	at android.os.Looper.loopOnce(Looper.java:201)
                                                                                                    	at android.os.Looper.loop(Looper.java:288)
                                                                                                    	at android.app.ActivityThread.main(ActivityThread.java:7842)
                                                                                                    	at java.lang.reflect.Method.invoke(Native Method)
                                                                                                    	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
                                                                                                    	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)

This happens if you try to search for "active" in the /post request to postman-echo.

Multiple occurrences in the same line are not handled properly. Here they're shown as 3 but they should be 5. Highlightning is also off.

You can reproduce this on the /xml request

4shutosh · 2023-04-29T21:50:05Z

Haven't tried to find a solution for point 2 yet, but not able to figure out 1 after quite a few tries. It happens only after we've done the search once in a transaction fragment, go back, and go to another request transaction screen. I believe something wrong with the instance of the fragment, or while making the copy of listOfScrollableIndex.

Any help regarding the same will be highly appreciated.

…line

4shutosh · 2023-04-30T10:53:24Z

Both the points are fixed now @cortinico.

Implemented substring level highlighting for the body lines of the response.

My doubt regarding the misbehaving fragment instance was correct. BEHAVIOR_RESUME_ONLY_CURRENT_FRAGMENT was interfering with lazy variables initialization inside the fragment. I've updated the deprecated FragmentStatePagerAdapter to viewPager2 & FragmentStateAdapter.

Please review, and let me know if any other changes are required.

vbuberen · 2023-04-30T13:40:01Z

Hey there. I am joining a bit late, but would like to leave some general feedback.

I believe that search results should appear under tabs instead of making tabs jump down. For example, see how my version of search looked like:
We specifically skipped an update to ViewPager2 because it had issues. Since there were no more stable version updates I believe that the issue still exists unless some other dependency like Fragment had to do something with it.
See for a reference: Migration to ViewPager 2 #159 (comment)

cortinico · 2023-04-30T17:37:20Z

My doubt regarding the misbehaving fragment instance was correct. BEHAVIOR_RESUME_ONLY_CURRENT_FRAGMENT was interfering with lazy variables initialization inside the fragment. I've updated the deprecated FragmentStatePagerAdapter to viewPager2 & FragmentStateAdapter.

Please let's do this in a separate PR as @vbuberen mentioned

4shutosh · 2023-05-03T12:17:31Z

Reverted to old viewpager implemented. Search summary moved to fragment, below the tab layout.

I suspect some misbehaving highlighting on the requests page, please check if possible. Will test it, and fix if found something.

Please take a look @cortinico, let me know if any changes are required.

cortinico

Please take a look @cortinico, let me know if any changes are required.

Looks great to me @4shutosh
I've done some manual testing and all looks great to me!

trietbui85 · 2023-05-21T02:27:12Z

Awesome @4shutosh @cortinico .
Is it the last PR before release v4.0.0?

4shutosh added 2 commits March 18, 2023 16:39

scroll on search added

eb31f8e

scroll to search item added

06b7c15

4shutosh requested a review from a team as a code owner March 18, 2023 11:28

detekt fix code improvement

57127d1

ArjanSM reviewed Apr 10, 2023

View reviewed changes

cortinico requested changes Apr 23, 2023

View reviewed changes

4shutosh added 2 commits April 29, 2023 17:33

fab removed, search summary added in appbar, coroutines used

42a303b

removed unused string, green bg colour added for search navigation item

29825b9

cortinico reviewed Apr 29, 2023

View reviewed changes

cortinico requested a review from ArjanSM April 29, 2023 14:49

conflicts resolved

6f7401f

code cleanup

20db3fa

viewpager updated, highlighting implemented for substrings in single …

e07a207

…line

better param names

1271a7f

4shutosh requested a review from cortinico April 30, 2023 11:39

cortinico added this to the 4.0.0 milestone May 3, 2023

cortinico mentioned this pull request May 3, 2023

What is the blocking issues of release 4.0.0? #1029

Closed

reverted to viewpager, search summary moved to fragment

dbff4bc

cortinico approved these changes May 20, 2023

View reviewed changes

cortinico merged commit aacd5ff into ChuckerTeam:main May 20, 2023

4shutosh mentioned this pull request Jul 13, 2023

Bug fix - Scroll to Search Navigation Bar Visibility Fix #1076

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatic Scroll on Search added #988

Automatic Scroll on Search added #988

4shutosh commented Mar 18, 2023 •

edited

Loading

4shutosh commented Mar 18, 2023

4shutosh commented Mar 20, 2023

cortinico commented Apr 10, 2023

ArjanSM left a comment •

edited

Loading

ArjanSM Apr 10, 2023 •

edited

Loading

cortinico Apr 23, 2023

4shutosh Apr 23, 2023

cortinico left a comment

cortinico Apr 23, 2023

cortinico Apr 23, 2023

4shutosh Apr 23, 2023

cortinico Apr 23, 2023

4shutosh Apr 23, 2023

cortinico Apr 29, 2023

4shutosh Apr 29, 2023

cortinico commented Apr 29, 2023

4shutosh commented Apr 29, 2023

4shutosh commented Apr 29, 2023

cortinico commented Apr 29, 2023

4shutosh commented Apr 29, 2023

4shutosh commented Apr 30, 2023

vbuberen commented Apr 30, 2023

cortinico commented Apr 30, 2023

4shutosh commented May 3, 2023

cortinico left a comment

trietbui85 commented May 21, 2023

	* Added scroll to highlighted text search in response screen [#988]eye
	* Added scroll to highlighted text search in response screen [#988]

Automatic Scroll on Search added #988

Automatic Scroll on Search added #988

Conversation

4shutosh commented Mar 18, 2023 • edited Loading

📷 Screenshots

📄 Context

📝 Changes

📎 Related PR

🚫 Breaking

🛠️ How to test

⏱️ Next steps

4shutosh commented Mar 18, 2023

4shutosh commented Mar 20, 2023

cortinico commented Apr 10, 2023

ArjanSM left a comment • edited Loading

Choose a reason for hiding this comment

ArjanSM Apr 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cortinico left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cortinico commented Apr 29, 2023

4shutosh commented Apr 29, 2023

4shutosh commented Apr 29, 2023

cortinico commented Apr 29, 2023

4shutosh commented Apr 29, 2023

4shutosh commented Apr 30, 2023

vbuberen commented Apr 30, 2023

cortinico commented Apr 30, 2023

4shutosh commented May 3, 2023

cortinico left a comment

Choose a reason for hiding this comment

trietbui85 commented May 21, 2023

4shutosh commented Mar 18, 2023 •

edited

Loading

ArjanSM left a comment •

edited

Loading

ArjanSM Apr 10, 2023 •

edited

Loading