Skip to content

Promo screenshots action refinements #685

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

Open
mokagio wants to merge 4 commits into
base: trunk
Choose a base branch
from
+19 −6
Open

Promo screenshots action refinements #685

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3c4eea8
Silence the check for `drawText` in the path
mokagio Jan 15, 2026
922a8ba
Suggests runing screenshots generation when file not found
mokagio Jan 15, 2026
95ed576
Address deprecation warning in RMagick
mokagio Jan 15, 2026
5740c60
Use `system` over `Action.sh` to call `drawText`
mokagio Jan 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions lib/fastlane/plugin/wpmreleasetoolkit/helper/promo_screenshots_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def initialize
UI.user_error!(message)
end

UI.user_error!('`drawText` not found – install it using `brew install automattic/build-tools/drawText`.') unless system('command -v drawText')
UI.user_error!('`drawText` not found – install it using `brew install automattic/build-tools/drawText`.') unless system('command -v drawText > /dev/null')
Copy link
Contributor Author

@mokagio mokagio Jan 16, 2026

Choose a reason for hiding this comment

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

Otherwise, we'd get a long list of command -v drawText output.

As a side note, this line seems to be called many times. We should cache, memoize, or otherwise implement a way to run this check only once.

end

def read_config(config_file_path)
Expand Down Expand Up @@ -287,7 +287,16 @@ def draw_text_to_canvas(canvas, text, width, height, x_position, y_position, fon
begin
temp_text_file = Tempfile.new

Action.sh('drawText', "html=#{text}", "maxWidth=#{width}", "maxHeight=#{height}", "output=#{temp_text_file.path}", "fontSize=#{font_size}", "stylesheet=#{stylesheet_path}", "alignment=#{position}")
system(
'drawText',
"html=#{text}",
"maxWidth=#{width}",
"maxHeight=#{height}",
"output=#{temp_text_file.path}",
"fontSize=#{font_size}",
"stylesheet=#{stylesheet_path}",
"alignment=#{position}"
)
Comment on lines -290 to +299
Copy link
Contributor Author

@mokagio mokagio Jan 16, 2026

Choose a reason for hiding this comment

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

Fastlane's sh should support logs: false to silence the call, but it did not parse.

Copy link
Contributor

@iangmaia iangmaia Jan 16, 2026

Choose a reason for hiding this comment

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

Shouldn't it be just log: false, per docs and source?


text_content = open_image(temp_text_file.path).trim
text_frame = create_image(width, height)
Expand Down Expand Up @@ -407,8 +416,8 @@ def create_image(width, height, background = 'transparent')
working_background = background.frozen? ? background.dup : background
working_background.paint.to_hex

Image.new(width, height) do
self.background_color = working_background
Image.new(width, height) do |info|
info.background_color = working_background
end
end
Comment on lines -410 to 422
Copy link
Contributor Author

@mokagio mokagio Jan 16, 2026

Choose a reason for hiding this comment

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

This addresses a deprecation warning. See commit message for more details.


Expand All @@ -432,8 +441,12 @@ def resolve_path(path)
return resolved_path if !resolved_path.nil? && resolved_path.exist?
end

message = "Unable to locate #{path}"
UI.crash!(message)
message = <<~MESSAGE
Unable to locate #{path}.

Did you run the automation to generate the screenshots?
Copy link
Contributor Author

@mokagio mokagio Jan 16, 2026

Choose a reason for hiding this comment

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

Just in case I go through the same process of pondering why annotating non existing screenshots fails again.

MESSAGE
UI.user_error!(message)
Copy link
Contributor Author

@mokagio mokagio Jan 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Used user_error! instead of crash! because it makes less noise in the logs as it doesn't give a stacktrace.

Copy link
Contributor

@iangmaia iangmaia Jan 16, 2026

Choose a reason for hiding this comment

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

Good one 👍

end

def resolve_text_into_path(text, locale)
Expand Down