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.

Sign up for GitHub

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

Jump to bottom

[frameit] fix device detection that would match less specific devices first #20642

Merged
merged 3 commits into from Oct 25, 2022
Merged

[frameit] fix device detection that would match less specific devices first #20642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e507e72
fix
guidev Sep 10, 2022
c27f054
fix tests
guidev Sep 10, 2022
8333937
Merge branch 'fastlane:master' into frameit_fix_device_selection
guidev Sep 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion frameit/lib/frameit/device.rb
View file
Open in desktop
Expand Up @@ -51,7 +51,7 @@ def self.detect_device(path, platform)
found_device = nil
filename_device = nil
filename = Pathname.new(path).basename.to_s
Devices.constants.each do |c|
Devices.constants.sort_by(&:length).reverse_each do |c|
Copy link
Member

Choose a reason for hiding this comment

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

This is a crazy assumption here. But quite safe, IMO. It doesn't rely on business logic or marketing naming standards, but rather "the longer the string, the more specific it will be" and thus matching with partial substrings won't be possible. Quite clever IMO :)

device = Devices.const_get(c)
next unless device.resolutions.include?(size)
# assign to filename_device if the filename contains the formatted name / id and its priority is higher than the current filename_device
Expand Down
4 changes: 2 additions & 2 deletions frameit/spec/device_spec.rb
View file
Open in desktop
Expand Up @@ -34,8 +34,8 @@ def expect_forced_screen_size(value)
end

it "should detect iPhone 13 in portrait and landscape based on priority" do
expect_screen_size_from_file("screenshot-Portrait{1170x2532}.jpg", Platform::IOS).to eq(Devices::IPHONE_13)
expect_screen_size_from_file("screenshot-Landscape{2532x1170}.jpg", Platform::IOS).to eq(Devices::IPHONE_13)
expect_screen_size_from_file("screenshot-Portrait{1170x2532}.jpg", Platform::IOS).to eq(Devices::IPHONE_13_PRO)
expect_screen_size_from_file("screenshot-Landscape{2532x1170}.jpg", Platform::IOS).to eq(Devices::IPHONE_13_PRO)
end

it "should detect iPhone X instead of iPhone XS because of the file name containing device name" do
Expand Down