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

Fix Call popup width issue on orientation change #13723

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mohsin363
Copy link

@mohsin363 mohsin363 commented Oct 3, 2024

Fixes Issue #13658: Call popup window has incorrect width when flipping phone after opening a chat #13658

First time contributor checklist

Contributor checklist

  • Xiaomi Redmi 10, Android 14
  • My contribution is fully baked and ready to be merged as is
  • I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit using the Fixes #1234 syntax

Description

This PR fixes the issue where the call popup/dialog size would not adjust when the orientation changed, causing it to remain at the size set when the page was first opened, which appeared awkward. Now, after the fix, the dialog/popup will resize appropriately based on the current orientation, regardless of the initial orientation when the page was opened.

How to test:

  1. Open a chat (on both Portrait and Landscape modes)
  2. Click on Call Icon
  3. Change Orientation

Here's screen recording video after the fix:
https://github.com/user-attachments/assets/9c4e63c1-d300-4804-9916-96395cb01a1f

Fixed Issue signalapp#13658: Call popup window has incorrect width when flipping phone after opening a chat signalapp#13658
@mohsin363 mohsin363 changed the title Fixes #13658 call popup width issue on orientation change Fix Call popup width issue on orientation change Oct 3, 2024
@mohsin363
Copy link
Author

hey @greyson-signal, could you review this PR? This fixes a really annoying dialog experience on orientation changes in production app. Any feedback on this would be greatly appreciated!

@greyson-signal
Copy link
Contributor

Thank you for taking the time to look into this! Looking at your code though, it just seems like a lot of stuff going on to fix this single dialog. And tbh, we have this problem with several dialogs. I believe it's a larger issue related to how the app handles configuration changes, and I'd rather attack the problem from the root rather than try to do a bunch of manual sizing fixes on a case-by-case basis.

@mohsin363
Copy link
Author

mohsin363 commented Dec 16, 2024

Thank you for taking the time to look into this! Looking at your code though, it just seems like a lot of stuff going on to fix this single dialog. And tbh, we have this problem with several dialogs. I believe it's a larger issue related to how the app handles configuration changes, and I'd rather attack the problem from the root rather than try to do a bunch of manual sizing fixes on a case-by-case basis.

@greyson-signal yeah you're right, I think this should be fixed for all other dialogs as well. And in my opinion, to handle configuration changes properly and to follow the best practices, dialogs should only be created inside a Fragment/Activity.

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

Successfully merging this pull request may close these issues.

2 participants