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

[frameit] [frames_generator] update for iPhone 14 #20917

Closed
wants to merge 3 commits into from

Conversation

guidev
Copy link
Contributor

@guidev guidev commented Dec 7, 2022

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I see several green ci/circleci builds in the "All checks have passed" section of my PR (connect CircleCI to GitHub if not)
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

Update for iPhone 14

depends on fastlane/frameit-frames#31

@guidev guidev changed the title update for iPhone 14 [frameit] [frames_generator] update for iPhone 14 Dec 9, 2022
@getaaron
Copy link
Collaborator

@guidev same on this PR, you need to auth with CircleCI and rebuild so the checks run

@guidev
Copy link
Contributor Author

guidev commented Dec 14, 2022

Hi @getaaron, this is what I see... Am I missing something?
Screenshot 2022-12-14 at 15 38 36

@nickkaczmarek
Copy link

Can't wait to see this get merged. Thanks @guidev and @getaaron

@schulzp
Copy link

schulzp commented Mar 2, 2023

Hi! Thanks for the great work! I highly appreciate it. 🙏🏻 Any chance, this PR could get reviewed (and merged)?

@barringb
Copy link

Bump, hoping for iPhone 14 support for app store 🙏

@Damien-B
Copy link

Waiting for the review and merge as well.
@guidev you might want to add some reviewers to this PR

@DigitalRogues
Copy link

DigitalRogues commented Jul 21, 2023

With the iPhone 15 normally coming out in a couple months it’d sure be nice to get support for the iPhone 14 that’s been out for almost a year. 😆

Copy link

@ricsantos ricsantos left a comment

Choose a reason for hiding this comment

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

LGTM

@ricsantos
Copy link

@joshdholtz would you be able to have a quick peek at this? needs a review to get merged, would be great to have support for the dynamic island screenshots...

@andreialecu
Copy link

@joshdholtz version 2.217.0 says that support for iPhone 14 devices has been added, but only the images are added. device_types.rb doesn't seem to have been updated. I believe this PR also needs to be merged and a new release cut.

I'm still getting [09:56:37]: Unsupported screen size [1290, 2796] for path './en-US/iPhone 15 Pro Max - 1 - Home.png' otherwise.

Note how iPhone 14 is missing here: https://github.com/fastlane/fastlane/blob/master/frameit/lib/frameit/device_types.rb#L131-L137

@dahchon
Copy link

dahchon commented Nov 18, 2023

@joshdholtz version 2.217.0 says that support for iPhone 14 devices has been added, but only the images are added. device_types.rb doesn't seem to have been updated. I believe this PR also needs to be merged and a new release cut.

I'm still getting [09:56:37]: Unsupported screen size [1290, 2796] for path './en-US/iPhone 15 Pro Max - 1 - Home.png' otherwise.

Note how iPhone 14 is missing here: https://github.com/fastlane/fastlane/blob/master/frameit/lib/frameit/device_types.rb#L131-L137

Same here. We need this PR.
There could be small fixes needed because I'm getting black screens.

@sathoeni sathoeni mentioned this pull request Dec 14, 2023
6 tasks
@sathoeni
Copy link
Contributor

IMHO the current device size for an iPhone 14 Pro is not correct in the current master of https://github.com/fastlane/frameit-frames/

Therefore I've created a pull-request with the correct frame size
I've also adapted the logic of framing, to apply rounded corners when framing an iPhone 14.

Maybe you can try follow fastlane-branch that links to a corresponding frame-it/frames repo that matches the correct iPhone 14 Pro size
https://github.com/sathoeni/fastlane/tree/customFrames

If you are fine with the implementation we can close all other pull requests and hopefully it's getting merged

.gsub("Apple", "")
.gsub("-", " ")
.gsub(' - ', ' ') # Google Pixel device names are separated from their colors by a dash -> remove
.gsub(' – ', ' ') # some Apple devices are separated from their colors by this weird dash -> remove
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.gsub(' – ', ' ') # some Apple devices are separated from their colors by this weird dash -> remove
.gsub(' – ', ' ') # some Apple devices are separated from their colors by an en dash -> remove

.gsub("-", " ")
.gsub(' - ', ' ') # Google Pixel device names are separated from their colors by a dash -> remove
.gsub(' – ', ' ') # some Apple devices are separated from their colors by this weird dash -> remove
.gsub(' — ', ' ') # some Apple devices are separated from their colors by this weird dash -> remove
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.gsub(' — ', ' ') # some Apple devices are separated from their colors by this weird dash -> remove
.gsub(' — ', ' ') # some Apple devices are separated from their colors by an em dash -> remove

@getaaron
Copy link
Collaborator

@guidev can you please commit my two suggestions or give me write permission to your branch? Then I'll merge this.

@sathoeni
Copy link
Contributor

@guidev can you please commit my two suggestions or give me write permission to your branch? Then I'll merge this.

@getaaron You shoul merge following pull request, it contains everything of this one and even more, like the rounded corners and correct frame size

#21727

@getaaron
Copy link
Collaborator

Merged this and additional improvements in #21727. Thank you everyone for the PR, discussions, contributions, and patience.

@getaaron getaaron closed this Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet