frameit-frames icon indicating copy to clipboard operation
frameit-frames copied to clipboard

Nexus 5x should be have capital X

Open RobertSasak opened this issue 4 years ago • 7 comments

I experience the same error as described here https://github.com/fastlane/frameit-frames/pull/10#discussion_r372636465

I believe there is a typo in letter X.

RobertSasak avatar Jun 19 '20 06:06 RobertSasak

I created a quick PR https://github.com/fastlane/frameit-frames/pull/11

RobertSasak avatar Jun 19 '20 07:06 RobertSasak

So the file is X but the offset definition is x right now?

janpio avatar Jun 19 '20 09:06 janpio

I think the correct one is capital X.

  • Nexus 6P has a capital P.
  • Here is also capital X https://github.com/fastlane/fastlane/blob/9075249574c33b0f28735ac2003078d8db52260d/frameit/lib/frameit/device_types.rb
  • And documentation also mention it with capital X. https://github.com/fastlane/fastlane/blob/9075249574c33b0f28735ac2003078d8db52260d/fastlane/lib/fastlane/actions/docs/frame_screenshots.md

Does it mean that I need to rename it

  • in actual PNG file as well Nexus 5x.png? https://github.com/RobertSasak/frameit-frames/blob/gh-pages/latest/Nexus%205x.png
  • and also in files.json? https://github.com/fastlane/frameit-frames/blob/gh-pages/latest/files.json

RobertSasak avatar Jun 19 '20 09:06 RobertSasak

That is what I do not know :) Seems this is all kinds of interconnected.

janpio avatar Jun 19 '20 11:06 janpio

It looks like renaming both the PNG file to Nexus 5X.png and the name in offsets.json to Nexus 5X fixes the issue with this particular device.

Given the frames files in repo frameit-frames might be used elsewhere, it might be easier to just rename with a lower case X in this file https://github.com/fastlane/fastlane/blob/master/frameit/lib/frameit/device_types.rb

davidmarquis avatar Sep 14 '20 13:09 davidmarquis

why is the PR still not merged, It would be better to be fixed ASAP

gelodgreat avatar Oct 31 '20 14:10 gelodgreat

I just ran into this problem now as well. I just changed the following and it works:

    "Nexus 5x": {
      "offset": "+53+231",
      "width": 1080
    },

to

    "Nexus 5X": {
      "offset": "+53+231",
      "width": 1080
    },

Not sure why we can't get this merged?

barnaclebarnes avatar Dec 14 '20 19:12 barnaclebarnes