Skip to content

Commit

Permalink
Fix oppia#5615 : Scroll Position For Policies Screen On Orientation C…
Browse files Browse the repository at this point in the history
…hange (oppia#5616)

## Explanation
This PR fixes oppia#5615 the issue where the scroll position in the policies
screen (Privacy Policy/Terms of Service) would reset after an
orientation change.

### **Changes Made:**
1. **Updated `PoliciesFragmentPresenter`:**
- Added a `scrollPosition` property to save and restore the scroll
state.
- Implemented `onSaveInstanceState` and `onRestoreInstanceState`
handling using a constant key.
- Ensured the scroll position is retained using
`ScrollView.viewTreeObserver` in `onGlobalLayoutListener`.

2. **Modified `PoliciesFragment`:**
   - Delegated `onSaveInstanceState` calls to the presenter.
- Passed the saved instance state to the presenter during fragment view
creation.

### **Testing Performed:**
- Verified that scroll position is correctly retained:
  - Across orientation changes (portrait ↔️ landscape).
  - On both Privacy Policy and Terms of Service screens
---

## Essential Checklist
- [x] The PR title and explanation each start with "Fix #bugnum: ".
- [x] No unnecessary code changes or Android Studio formatting changes
are included.
- [x] The branch is **not** called "develop" and is up-to-date with
"develop".
- [x] Appropriate reviewers are assigned.

---

After Changes :

[Screen_recording_20241223_010758.webm](https://github.com/user-attachments/assets/f9648b03-5b27-4090-9eaa-82946214cd62)

---------

Co-authored-by: Adhiambo Peres <[email protected]>
  • Loading branch information
TanishMoral11 and adhiamboperes authored Jan 22, 2025
1 parent 68f20cb commit 8ce374b
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ class PoliciesFragment : InjectableFragment() {
POLICIES_FRAGMENT_POLICY_PAGE_ARGUMENT_PROTO,
PoliciesFragmentArguments.getDefaultInstance()
)
return policiesFragmentPresenter.handleCreateView(inflater, container, policies)
return policiesFragmentPresenter
.handleCreateView(inflater, container, policies, savedInstanceState)
}

override fun onSaveInstanceState(outState: Bundle) {
super.onSaveInstanceState(outState)
policiesFragmentPresenter.handleSaveInstanceState(outState)
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package org.oppia.android.app.policies

import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.widget.ScrollView
import androidx.appcompat.app.AppCompatActivity
import org.oppia.android.R
import org.oppia.android.app.fragment.FragmentScope
Expand All @@ -22,21 +24,41 @@ class PoliciesFragmentPresenter @Inject constructor(
private val resourceHandler: AppLanguageResourceHandler
) : HtmlParser.PolicyOppiaTagActionListener {

private lateinit var binding: PoliciesFragmentBinding
private var scrollPosition = 0

/** Handles onCreate() method of the [PoliciesFragment]. */
fun handleCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
policiesFragmentArguments: PoliciesFragmentArguments
policiesFragmentArguments: PoliciesFragmentArguments,
savedInstanceState: Bundle?
): View {
val binding = PoliciesFragmentBinding.inflate(
binding = PoliciesFragmentBinding.inflate(
inflater,
container,
/* attachToRoot= */ false
)

savedInstanceState?.let {
scrollPosition = it.getInt(KEY_SCROLL_Y, 0)
}

setUpContentForTextViews(policiesFragmentArguments.policyPage, binding)

(binding.root as ScrollView).viewTreeObserver.addOnGlobalLayoutListener {
binding.root.scrollTo(0, scrollPosition)
}

return binding.root
}
/**
* Saves the current scroll position of the policies page into the given [outState].
* This helps in restoring the scroll position after orientation changes.
*/
fun handleSaveInstanceState(outState: Bundle) {
outState.putInt(KEY_SCROLL_Y, (binding.root as ScrollView).scrollY)
}

private fun setUpContentForTextViews(
policyPage: PolicyPage,
Expand Down Expand Up @@ -86,4 +108,8 @@ class PoliciesFragmentPresenter @Inject constructor(
(activity as RouteToPoliciesListener).onRouteToPolicies(PolicyPage.TERMS_OF_SERVICE)
}
}

companion object {
private const val KEY_SCROLL_Y = "policies_scroll_y"
}
}
1 change: 1 addition & 0 deletions app/src/main/res/layout/policies_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
xmlns:app="http://schemas.android.com/apk/res-auto">

<ScrollView
android:id="@+id/policy_scroll_view"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:background="@color/component_color_shared_screen_tertiary_background_color"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import android.app.Application
import android.app.Instrumentation.ActivityResult
import android.content.Context
import android.content.Intent
import android.content.pm.ActivityInfo
import android.text.Spannable
import android.text.style.ClickableSpan
import android.widget.ScrollView
import android.widget.TextView
import androidx.appcompat.app.AppCompatActivity
import androidx.test.core.app.ActivityScenario.launch
Expand Down Expand Up @@ -304,6 +306,34 @@ class PoliciesFragmentTest {
}
}

@Test
fun testPoliciesFragment_privacyPolicy_retainsScrollPositionAfterOrientationChange() {
launch<PoliciesFragmentTestActivity>(
createPoliciesFragmentTestActivity(
getApplicationContext(),
PolicyPage.PRIVACY_POLICY
)
).use { scenario ->
scenario.onActivity { activity ->
val scrollView = activity.findViewById<ScrollView>(R.id.policy_scroll_view)
scrollView.scrollTo(0, 500)
testCoroutineDispatchers.runCurrent()

// Capture the current scroll position.
val initialScrollPosition = scrollView.scrollY
assertThat(initialScrollPosition).isEqualTo(500)

// Rotate the screen (simulate an orientation change).
activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE
testCoroutineDispatchers.runCurrent()

// Verify that the scroll position is retained after the orientation change.
val newScrollPosition = scrollView.scrollY
assertThat(newScrollPosition).isEqualTo(initialScrollPosition)
}
}
}

@Test
fun testPoliciesFragment_checkTermsOfServiceWebLink_opensTheLink() {
launch<PoliciesFragmentTestActivity>(
Expand Down

0 comments on commit 8ce374b

Please sign in to comment.