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

Bugfix: Fix top padding #1010

Merged
merged 4 commits into from
Apr 4, 2024
Merged

Conversation

wutschel
Copy link
Collaborator

Description

Fixes a regression caused by #889.

Generally it is correct to use UIApplication.sharedApplication.keyWindow.safeAreaInsets.top in getTopPadding. This resolves issues with moving and wrongly positioned tables in iPhone main menu and custom button view. For the HostManagementViewController we use UIApplication.sharedApplication.statusBarFrame.size.height to read the size of the status bar.

Summary for release notes

Bugfix: Fix vertical origin of main menu and custom button view

@wutschel
Copy link
Collaborator Author

Conflicts with #1004. Please prioritize #1004, I will then rebase this one.

@wutschel wutschel marked this pull request as draft March 26, 2024 06:05
@wutschel
Copy link
Collaborator Author

wutschel commented Mar 26, 2024

Converted back to draft as I obviously should relook the approach. It seems I need two different padding values: one when showing a navbar (= status bar height + navbar height) and another when not (= safe area height). For this I need to revisit the code as some views have a navbar and some have not.

@wutschel wutschel force-pushed the fix_toppadding branch 4 times, most recently from ae37b48 to 4fd2628 Compare April 2, 2024 18:13
@wutschel
Copy link
Collaborator Author

wutschel commented Apr 2, 2024

I am sure you will stumble over this one:

deltaY = 44 + UIApplication.sharedApplication.statusBarFrame.size.height;

The 44 is the NavigationBar's height which is 0 during init time. I am not sure why moving the messagesView layout to viewDidLoad does result in invisible messages, For now let us please keep it like this, I anyway have an issue open to rework the whole SettingsValuesVC which is the nastiest piece of code left from the past.

@wutschel wutschel marked this pull request as ready for review April 2, 2024 18:29
@kambala-decapitator
Copy link
Collaborator

it's very simple: in viewDidLoad views don't have their final size. The best place to perform layout is viewDidLayoutSubviews

@wutschel
Copy link
Collaborator Author

wutschel commented Apr 2, 2024

viewDidLayoutSubviews

Works. I am sure this is still done wrong in many places ... So much work to do ...

@wutschel wutschel force-pushed the fix_toppadding branch 2 times, most recently from a6e4e4a to 48a0852 Compare April 2, 2024 20:45
XBMC Remote/Utilities.m Show resolved Hide resolved
XBMC Remote/SettingsValuesViewController.m Outdated Show resolved Hide resolved
@wutschel wutschel force-pushed the fix_toppadding branch 2 times, most recently from f2f50e7 to 7a70894 Compare April 3, 2024 18:09
Resolves an issue with iPhones which resulted in violation of safe area. This was leading to shifting vertical origin of tables in main menu and custom button view.
For HostManagementVC and NowPlaying the height of status and navigation bar needs to be used as this defines the vertical padding. A dedicated method is introduced to get this padding while having a navigation bar.
Separate MessagesView's creation from frame updates. Remove visually non-relevant border.
This allows to make use of getTopPaddingWithNavBar instead of a magic number during layout.
@wutschel
Copy link
Collaborator Author

wutschel commented Apr 3, 2024

Squashed and ready to go.

@kambala-decapitator kambala-decapitator merged commit 7841f3a into xbmc:master Apr 4, 2024
@wutschel wutschel deleted the fix_toppadding branch April 4, 2024 18:23
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.

None yet

2 participants