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.

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

Fix Crash in BottomSheetViewController due to Incorrect Selector Usage #128

Merged
merged 4 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ _None._

_None._

## 1.13.1

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.


### New Features

Choose a reason for hiding this comment

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

Nit: This is a bug fix, so please note it with Bug Fixes instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


- Addresses a crash in the `BottomSheetViewController` that was occurring 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`.

## 1.13.0

### New Features
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ public class BottomSheetViewController: UIViewController {
fatalError("init(coder:) has not been implemented")
}

private var gripButton: UIButton = {
private lazy var gripButton: UIButton = {
let button = GripButton()
button.translatesAutoresizingMaskIntoConstraints = false
button.addTarget(
BottomSheetViewController.self,
self,
action: #selector(buttonPressed),
for: .touchUpInside
)
Expand Down
2 changes: 1 addition & 1 deletion WordPressUI.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

Pod::Spec.new do |s|
s.name = 'WordPressUI'
s.version = '1.13.0'
s.version = '1.13.1'

Choose a reason for hiding this comment

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

Similar as above, this fix should be tagged 1.13.1-beta.

Copy link
Contributor

@mokagio mokagio Jul 19, 2023

Choose a reason for hiding this comment

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

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.

Choose a reason for hiding this comment

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

I don't have a strong opinion about this, so I'll leave the question to @pmusolino.

Copy link
Member Author

Choose a reason for hiding this comment

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

It would be beneficial to circumvent the beta phase, as the absence of this particular fix results in an app crash. @mokagio 🙌


s.summary = 'Home of reusable WordPress UI components.'
s.description = <<-DESC
Expand Down