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

goalVC now also respects safe area #495

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

krugerk
Copy link
Contributor

@krugerk krugerk commented Oct 20, 2024

goal view controller was not respecting the safe areas. This meant, for example, that the graph was partially hidden behind the Dynamic Island.

@theospears
Copy link
Collaborator

I've really appreciated the before/after screenshots you've included in some other PRs - could you add one here?

@krugerk
Copy link
Contributor Author

krugerk commented Oct 21, 2024

I've really appreciated the before/after screenshots you've included in some other PRs - could you add one here?

sure.

This is what it looks like broken / currently:

safe area left
safe area right

The data area though on the goal vc accounted for the safe area:
data area already respecting left and right safe area

thanks to

make.left.equalTo(self.goalImageScrollView).offset(sideMargin)
make.right.equalTo(self.goalImageScrollView).offset(-sideMargin)
and
make.left.greaterThanOrEqualTo(self.scrollView.safeAreaLayoutGuide.snp.leftMargin)
make.right.lessThanOrEqualTo(self.scrollView.safeAreaLayoutGuide.snp.rightMargin)

@theospears theospears merged commit 693fa6c into beeminder:master Oct 21, 2024
1 check passed
@theospears
Copy link
Collaborator

This seems reasonable, although I wonder if the app should even support landscape mode on phones

@krugerk
Copy link
Contributor Author

krugerk commented Oct 21, 2024

This seems reasonable, although I wonder if the app should even support landscape mode on phones

Yeah that could be worth the discussion. This has been broken since iPhone X! (Some apps do purposely have content scroll inside the safe area and behind the Dynamic Island or into the status bar area.)
The app is (still) somewhat inconsistent about these sorts of things. Currently I am thinking we just iteratively clean it up, at least until the sweeping overhaul.

@krugerk krugerk deleted the bugfix/goalVCIgnoringSafeArea branch October 21, 2024 12:27
@krugerk
Copy link
Contributor Author

krugerk commented Oct 21, 2024

mentioning #64, a table exclusive layout

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.

2 participants