Prevent alerts to be captured

fastlane-old / snapshot

Automate taking localized screenshots of your iOS app on every device

https://fastlane.tools

1.95k stars 140 forks source link

Prevent alerts to be captured #454

Closed maxbritto closed 8 years ago

maxbritto commented 8 years ago

Calls to the snapshot() function will now wait until all alert have disappeared before taking the screenshot (can be disabled with a boolean parameter on the snapshot() method)
Adds a function to allow automatic dismissal of alerts that prevent actions or snapshots
Copy the new SnapshotAlertManagement.swift file to the same place as the SnapshotHelper.swift file during original setup

koenpunt commented 8 years ago

Is the waitForAlerts argument necessary? Shouldn't it be that if automaticallyDismissAlerts is called, all alert are dismissed? And if someone doesn't want an alert to be dismissed, they simply call stopDismissingAlerts

koenpunt commented 8 years ago

Or if it stays, I think it should be named differently, because now it implies to wait for an alert to be shown, instead of the dismissal thereof

maxbritto commented 8 years ago

I'm not sure because waitForAlerts actually does wait but doesn't dismiss the alert by itself. It does the same thing as waitForLoadingIndicator. It lets the developer chose its own way of dismissing the alert : he can take care of it on it's own if the alert matters or call automaticallyDismissAlerts to automatically dismiss any alert that gets in the way.

maxbritto commented 8 years ago

Regarding your second comment, I see what you mean but then waitForLoadingIndicator should also be renamed.

koenpunt commented 8 years ago

Regarding your second comment, I see what you mean but then waitForLoadingIndicator should also be renamed.

You're right.

I'm not sure because waitForAlerts actually does wait but doesn't dismiss the alert by itself. It does the same thing as waitForLoadingIndicator.

waitForLoading indicator waits for the loading indicator to disappear, but you say that waitForAlerts doesn't dismiss alerts? That might be confusing to users

maxbritto commented 8 years ago

I tried to follow the pattern of waitForLoading and do the same : I just wait for the alert to disappear. This is based on the assumption that many developer are probably doing meaningful things with their alerts and do not want us to dismiss them as they appear. But I've also added a print if we have been waiting for more than 2 seconds and an alert is still here : it explains how to enable the automatic dismissal if they want it. I also added documentation in setup.rb

i2amsam commented 8 years ago

Hey @maxbritto thank you for this!

I'm not going to merge this for now, though I think this an interesting problem, I'm concerned about adding additional dependencies and the migration path for existing setups. This could be a good candidate for getting "local helpers" or "local action" for some sort of plugin system in snapshot so the helpers can be easily extended.

maxbritto commented 8 years ago

I've never seen any plugin system for fastlane/snapshot. If there is one I'd be willing to refactor this as a plug-in. But if you're asking me to help building the plugin system I'm not sure how efficient I would be since I've never done any Ruby.