Skip to content

Resolve unresponsiveness and state loss on Place type screen rotation#6685

Open
Roniscend wants to merge 1 commit intocommons-app:mainfrom
Roniscend:Place_Type
Open

Resolve unresponsiveness and state loss on Place type screen rotation#6685
Roniscend wants to merge 1 commit intocommons-app:mainfrom
Roniscend:Place_Type

Conversation

@Roniscend
Copy link
Contributor

Description (required)
Handled configuration changes to prevent UI freezing and ensure state retention in the Place type selector
Fixes #6676

Tested 6.3.0-debug on Pixel 9 (Android 16[API 36])

Copilot AI review requested due to automatic review settings February 26, 2026 13:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Addresses issue #6676 by attempting to preserve “Place type” filter UI state across configuration changes, preventing unresponsiveness/state loss after rotation.

Changes:

  • Track filter UI state (list visibility, query text, focus) in fragment fields.
  • Persist and restore that state via onSaveInstanceState() / onViewStateRestored().
  • Re-apply saved UI state during filter initialization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

* Restores the Place type filter UI state from the provided Bundle.
* This is called after configuration changes to restore the state.
*/
override fun onViewStateRestored(savedInstanceState: Bundle?) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about instead of onViewStateRestored? and get it from viewmodel and make it a single source of truth?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have view models here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn’t aware that introducing ViewModels was allowed, as it hadn’t been discussed earlier. I was strictly following the existing MVP architecture used in the project.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We'll be moving to MVVM soon, but I'm not expecting big migrations in the PRs. If it's a small change, please feel free to implement it. Please check the pinned issue on configuration changes for related discussions, thanks! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure I'll update my implementation then

@github-actions
Copy link

github-actions bot commented Mar 3, 2026

✅ Generated APK variants!

@Roniscend Roniscend requested a review from neeldoshii March 3, 2026 16:00
@Roniscend
Copy link
Contributor Author

@neeldoshii please can you re review this pr

Copy link
Collaborator

@neeldoshii neeldoshii left a comment

Choose a reason for hiding this comment

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

Hi @Roniscend ,
I am unable to reproduce the issue on main, can you help me to reproduce?
Thanks

Main branch

XRecorder_20260312_01.mp4

@neeldoshii
Copy link
Collaborator

Hi @Roniscend. just checking in do we have any updates on steps to reproduce?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Place type screen becomes unresponsive and state is lost after device rotation

5 participants