Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Ref #3838: Fix overall menu height on smaller devices #3850

Merged
merged 1 commit into from
Jun 25, 2021

Conversation

kylehickinson
Copy link
Collaborator

@kylehickinson kylehickinson commented Jun 25, 2021

PanModal uses topOffset to determine the height of the presenting controller rather than the longFormHeight, so adjusting where I add that 32pt's for accessibility to fix the bug caused by adding that 32pt's in the wrong location.

Summary of Changes

This pull request fixes a small bug caused by the #3845

Submitter Checklist:

  • Unit Tests are updated to cover new or changed functionality
  • User-facing strings use NSLocalizableString()

Test Plan:

  • Verify on devices that have no bottom safe area that toolbars are in the correct spot

Screenshots:

iPhone 6s iPhone 11 Pro
Simulator Screen Shot - iPhone 6s - 2021-06-25 at 10 09 43 Simulator Screen Shot - iPhone 11 Pro - 2021-06-25 at 10 09 49

Reviewer Checklist:

  • Issues include necessary QA labels:
    • QA/(Yes|No)
    • release-notes/(include|exclude)
    • bug / enhancement
  • Necessary security reviews have taken place.
  • Adequate unit test coverage exists to prevent regressions.
  • Adequate test plan exists for QA to validate (if applicable).
  • Issue is assigned to a milestone (should happen at merge time).

PanModal uses `topOffset` to determine the height of the presenting controller rather than the `longFormHeight`, so adjusting where I add that 32pt's for accessibility to fix the bug caused by adding that 32pt's in the wrong location.
@kylehickinson kylehickinson requested a review from iccub June 25, 2021 14:11
@kylehickinson kylehickinson added this to the 1.28 milestone Jun 25, 2021
@@ -41,7 +41,7 @@
"package": "PanModal",
"repositoryURL": "https://github.com/brave/PanModal",
"state": {
"branch": "master",
"branch": null,
Copy link
Contributor

Choose a reason for hiding this comment

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

this ok?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I kept seeing it pop up so I let it through. Its Xcode fixing up its own mistake, it should have null'd out the branch when I changed it to a commit, but it didnt for some reason

@iccub iccub merged commit e417f20 into development Jun 25, 2021
@iccub iccub deleted the fix-menu-height branch June 25, 2021 14:52
iccub pushed a commit that referenced this pull request Jun 25, 2021
PanModal uses `topOffset` to determine the height of the presenting controller rather than the `longFormHeight`, so adjusting where I add that 32pt's for accessibility to fix the bug caused by adding that 32pt's in the wrong location.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants