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

added new device "Pixel 4" #195

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Uuttssaavv
Copy link

No description provided.

@@ -163,6 +181,7 @@ class AndroidDevices {
sonyXperia1II,
smallPhone,
mediumPhone,
p4,

Choose a reason for hiding this comment

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

Why not rename p4 to pixel4? Look at the other names such as sonyXperia1II. Follow the same naming convention.

Copy link

@upstreetcoding upstreetcoding left a comment

Choose a reason for hiding this comment

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

Your PR has a few issues:

Your PR suggests that you're adding the Pixel 4 device, but you commit these files too. Leave these out:
device_frame/lib/src/devices/ios/iphone_12/device.dart
device_frame/example/ios/Runner/Info.plist
device_frame/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme
device_frame/example/ios/Runner.xcodeproj/project.pbxproj
.vscode/launch.json
If you're new with PR's its understandable. You should create a seperate branch, based on aloisdeniel:master, and only commit changes that your PR is about.

Second, from looking at your PR, there is no device frame for the Pixel 4 added. You're just using a generic frame and changed the values to match that of the Pixel 4. I dont see much value in this PR because everyone can create a custom device with DeviceInfo.genericPhone()

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