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

Add new iPhone Models to screenHeights.js #102

Merged
merged 7 commits into from
Oct 13, 2022

Conversation

spencersablan
Copy link
Contributor

Summary

  • Needed to add iPhone 14 Pro to screenHeights.js since the Venmo button was not showing up on that device.
  • Also added a couple models that I found were missing just for consistency 😄

Conversation

  • I'd love to document how we find this data. I'm thinking of including that information in another PR, since we would have to find a way to organize our docs in this repo.

@spencersablan spencersablan requested a review from a team as a code owner October 11, 2022 20:55
@mnicpt
Copy link
Contributor

mnicpt commented Oct 12, 2022

@spencersablan Also need to add iPhone 14 Pro Max. It has an outerHeight of 932.

@mnicpt
Copy link
Contributor

mnicpt commented Oct 12, 2022

@spencersablan Can you also add iPhone 14 to the 844 section description? Here is a good link to make sure we are updated across the board. https://useyourloaf.com/blog/iphone-14-screen-sizes/

src/screenHeights.js Outdated Show resolved Hide resolved
@mnicpt
Copy link
Contributor

mnicpt commented Oct 12, 2022

@spencersablan Nice work by the way! This is not a fun task.

src/screenHeights.js Outdated Show resolved Hide resolved
Co-authored-by: Steven Mask <mnicpt@gmail.com>
@spencersablan
Copy link
Contributor Author

spencersablan commented Oct 12, 2022

@mnicpt Thank you for all your help & input! 😄

Yesterday, I went through & test every device that is iOS 16 compatible for whether or not the Venmo button is displaying as expected. I'll add my results below just so we are all on the same page. Interesting that, besides the new iPhones, we didn't have any that didn't show up in the Safari browser.

Action steps for myself today:

  • Add iPhone 14 Pro Max to screenHeights.js
  • Test iPhone 13 Mini, iPhone 12 Mini, iPhone 11 Pro Max, & iPhone XS Max to figure out why these might be displaying in an SFVC.
  • Test iPhone 14 Pro Safari browser textSizeHeights to make sure they don't overlap with SFVC textSizeHeights

Screen Shot 2022-10-11 at 4 59 06 PM

@spencersablan spencersablan requested a review from mnicpt October 13, 2022 16:29
@mnicpt
Copy link
Contributor

mnicpt commented Oct 13, 2022

@spencersablan Nice work!

@mnicpt mnicpt merged commit e3d8b32 into krakenjs:main Oct 13, 2022
westeezy pushed a commit that referenced this pull request Oct 13, 2022
* fix: add new iphone models to screenheights.js

* fix: add iphone 14

Co-authored-by: Steven Mask <mnicpt@gmail.com>

* fix: add iphone 14 pro max to screenheights.js

* fix: fix 1x safari browser overlap on iphone 14 pro

* fix: add iphone 12 mini specs

* fix: remove extra data for minis

Co-authored-by: Steven Mask <mnicpt@gmail.com>
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.

3 participants