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(hasNotch): Missing device name for iPhone 13 #1309

Merged
merged 4 commits into from
Sep 16, 2021

Conversation

rolfb
Copy link
Contributor

@rolfb rolfb commented Sep 16, 2021

Description

Adds missing device name by code for iPhone 13 lineup. Relates to #1307

Compatibility

OS Implemented
iOS

Adds missing device name by code for iPhone 13 lineup. Relates to react-native-device-info#1307
Copy link
Contributor

@schie schie left a comment

Choose a reason for hiding this comment

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

LGTM

@"iPhone14,1": @"iPhone 13 mini",
@"iPhone14,2": @"iPhone 13",
@"iPhone14,3": @"iPhone 13 Pro",
@"iPhone14,4": @"iPhone 13 Pro Max",
Copy link

@hiteshSavkare hiteshSavkare Sep 16, 2021

Choose a reason for hiding this comment

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

Hi @rolfb
for iPhone 13 the above combination is not working , whereas for other devices of 13 series its working as expected .
I tried with following combinations and looks me fine , i m not sure what is the number game of 1,2,3,4 vs 5,2,3,4.

    @"iPhone14,5": @"iPhone 13 mini",
    @"iPhone14,2": @"iPhone 13",
    @"iPhone14,3": @"iPhone 13 Pro",
    @"iPhone14,4": @"iPhone 13 Pro Max",

Sorry i am not IOS expert , just done some experiment with this numbers , so cant say anything about this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hiteshSavkare Great feedback, how exactly are you testing this?

Choose a reason for hiding this comment

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

@rolfb Thanks,
react-native-device-info into is integrated in my project, so changed the file RNDeviceInfo.m with above code and building the project with xcode 13 and testing on simulators of 13 series

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hiteshSavkare updated the file now, going to try to verify with the most recent xcode beta as well.

Copy link

@hiteshSavkare hiteshSavkare Sep 16, 2021

Choose a reason for hiding this comment

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

Sounds good @rolfb, thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hiteshSavkare Updated the list to match the device traits list in Xcode

sqlite> select ProductType,ProductDescription from Devices where ProductDescription LIKE 'iPhone 13%';
ProductType|ProductDescription
iPhone14,2|iPhone 13 Pro
iPhone14,3|iPhone 13 Pro Max
iPhone14,4|iPhone 13 mini
iPhone14,5|iPhone 13

Choose a reason for hiding this comment

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

Great @rolfb , Hope you tested with simulators or Do i need to test again ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hiteshSavkare didn't check as I expect Xcode's own lookup tables to be correct. Feel free to verify as the list from Xcode isn't exactly matching the one you provided.

Choose a reason for hiding this comment

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

@rolfb looks fine 👍

@rolfb
Copy link
Contributor Author

rolfb commented Sep 16, 2021

@schie @mikehardy please hold back on merging this for a little while, need to verify codes with Xcode RC

Updated device code to name mapping to match the information in the sqlite3 device traits database provided with Xcode RC.
@rolfb
Copy link
Contributor Author

rolfb commented Sep 16, 2021

@schie @mikehardy please hold back on merging this for a little while, need to verify codes with Xcode RC

Ok, I think we're good. Matching the exact values provided by Xcode RC itself from Contents/Developer/Platforms/iPhoneOS.platform/usr/standalone/device_traits.db

sqlite> select ProductType,ProductDescription from Devices where ProductDescription LIKE 'iPhone 13%';
ProductType|ProductDescription
iPhone14,2|iPhone 13 Pro
iPhone14,3|iPhone 13 Pro Max
iPhone14,4|iPhone 13 mini
iPhone14,5|iPhone 13

@mikehardy mikehardy merged commit 0bc979e into react-native-device-info:master Sep 16, 2021
@mikehardy
Copy link
Collaborator

Thank you all so much!

rndi-bot pushed a commit that referenced this pull request Sep 16, 2021
## [8.3.3](v8.3.2...v8.3.3) (2021-09-16)

### Bug Fixes

* **hasNotch:** Missing device name for iPhone 13 ([#1309](#1309)) ([0bc979e](0bc979e)), closes [#1307](#1307)
@mikehardy mikehardy linked an issue Sep 16, 2021 that may be closed by this pull request
@rndi-bot
Copy link
Collaborator

🎉 This PR is included in version 8.3.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@harisshk
Copy link

iPhone 13 series hasNotch() not working giving value as false

@mikehardy
Copy link
Collaborator

@harisshk https://stackoverflow.com/help/how-to-ask / https://stackoverflow.com/help/minimal-reproducible-example - I suspect you have outdated versions. Run our example from this repo on an iphone13 device and if you can reproduce open a new issue

@pckz
Copy link

pckz commented Feb 26, 2022

@harisshk same for me, tested in the simulator

@schie
Copy link
Contributor

schie commented Feb 26, 2022

@pckz what version are you on?

Like @mikehardy said to @harisshk, me thinks you're on an outdate version.

@pckz
Copy link

pckz commented Feb 26, 2022

@pckz what version are you on?

Like @mikehardy said to @harisshk, me thinks you're on an outdate version.

    "react-native": "0.62.2",
    "react-native-device-info": "^8.4.9",

(i've forced the latest version)

@mikehardy
Copy link
Collaborator

same for me, tested in the simulator

Tested what in the simulator? Your app? Our example? Testing our example app would be best, no telling what your app is doing, since you haven't provided any details of it

@pckz
Copy link

pckz commented Feb 26, 2022

same for me, tested in the simulator

Tested what in the simulator? Your app? Our example? Testing our example app would be best, no telling what your app is doing, since you haven't provided any details of it

My project runs:

"react-native": "0.62.2",
"react-native-device-info": "^8.4.9",

About the simulator, is Phone 13 running iOS 15.2. My XCode version is 13.2.1 (13C100)

I get false value doing this:

header :

 import DeviceInfo from 'react-native-device-info';

componentWillMount:

 console.log(DeviceInfo.hasNotch());

@mikehardy
Copy link
Collaborator

Cannot reproduce with example app.
image

WictorWilnd added a commit to WictorWilnd/react-native-device-info that referenced this pull request Feb 27, 2023
wdavis122 added a commit to wdavis122/react-native-device-info that referenced this pull request Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hasNotch don't work in Iphone 13
7 participants