Fix Crash in BottomSheetViewController due to Incorrect Selector Usage #128
Conversation
…ng due to an incorrect usage of a selector. The crash was caused by trying to call the `buttonPressed` instance method as a class method on `BottomSheetViewController.self`.
| @@ -48,6 +48,12 @@ _None._ | |||
|
|
|||
| _None._ | |||
|
|
|||
| ## 1.13.1 | |||
There was a problem hiding this comment.
Nit: Last time I checked, any fixes or updates to libraries should be set with a beta version, and release managers will take care of releasing a new version. So here we should use 1.13.1-beta-1 instead.
There was a problem hiding this comment.
FWIW, there is no need to update the version header in the CHANGELOG while in development. The release manager that will ship the next stable version will do that.
This is advantageous because it allows all different kind of changes to land in the main branch and for the semantic version value to be evaluated only at the time of cutting a new stable version.
Example: If only bug fixes landed, we shall make the new version a patch. But if a new feature landed as well as bug fixes, it should be a new minor version.
CHANGELOG.md
Outdated
| @@ -48,6 +48,12 @@ _None._ | |||
|
|
|||
| _None._ | |||
|
|
|||
| ## 1.13.1 | |||
|
|
|||
| ### New Features | |||
There was a problem hiding this comment.
Nit: This is a bug fix, so please note it with Bug Fixes instead.
| @@ -2,7 +2,7 @@ | |||
|
|
|||
| Pod::Spec.new do |s| | |||
| s.name = 'WordPressUI' | |||
| s.version = '1.13.0' | |||
| s.version = '1.13.1' | |||
There was a problem hiding this comment.
Similar as above, this fix should be tagged 1.13.1-beta.
There was a problem hiding this comment.
If it's convenient for the WooCommerce release schedule, we can bypass the beta step and ship a new stable version straightaway.
If, however, you'd rather ship a beta first, then this should be 1.31.1-beta.1
Let me know.
There was a problem hiding this comment.
I don't have a strong opinion about this, so I'll leave the question to @pmusolino.
There was a problem hiding this comment.
It would be beneficial to circumvent the beta phase, as the absence of this particular fix results in an app crash. @mokagio 🙌
|
@mokagio could I proceed with the merge? Is there anything missing? 🙇 |
This PR addresses a crash in the
BottomSheetViewControllerthat was occurring due to incorrect usage of a selector. The crash was caused by trying to call thebuttonPressedinstance method as a class method onBottomSheetViewController.self.Changes:
vartolazy varforgripButtonto delay its initialization until it's first used.BottomSheetViewController.selfwithselfinaddTargetmethod to correctly callbuttonPressedon the current instance ofBottomSheetViewController.These changes fix the crash and ensure that the
buttonPressedmethod is correctly called when thegripButtonis pressed. Thedismissfunctionality of the bottom sheet should now work as expected.Testing:
dismissfunctionality of the bottom sheet in various scenarios to confirm the fix. You can test it in this WooCommerce PR: Fix Crash inBottomSheetViewControllerwoocommerce/woocommerce-ios#10254CHANGELOG.mdif necessary.