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

Add boolean flag to disable auto updating send button enabled state #530

Merged
merged 5 commits into from
Feb 24, 2018

Conversation

clayellis
Copy link
Contributor

@clayellis clayellis commented Feb 20, 2018

What does this implement/fix? Explain your changes.

Adds a boolean flag to disable auto updating send button enabled state on text did change.

Does this close any currently open issues?

#529

Any other comments?

This PR doesn't change any current default behavior. Use of the flag is completely optional.

Where has this been tested?

Devices/Simulators: Both

iOS Version: iOS 11.2

Swift Version: 4

MessageKit Version: 0.13.1

@clayellis clayellis changed the title Add boolean flag to disabled auto updating send button enabled state … Add boolean flag to disable auto updating send button enabled state Feb 20, 2018
@@ -136,6 +136,9 @@ open class MessageInputBar: UIView {
$0.messageInputBar?.didSelectSendButton()
}
}()

/// A boolean that determines if the sendButton's `isEnabled` state should be auto updated on text changes.
open var shouldAutoUpdateSendButtonEnabledStateOnTextDidChange = true
Copy link
Member

Choose a reason for hiding this comment

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

I'm cool with this 😎 Thanks for making the PR. Can we rename it shouldManageSendButtonEnabledState? Open to shorter suggestions 😂

We also need to add a CHANGELOG entry under Added

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just following the naming conventions I saw used elsewhere for the sake of being consistent. (Specifically shouldAutoUpdateMaxTextViewHeight). I'm up for being more concise as long as it's not at the expense of clarity! shouldManageSendButtonEnabledState is great. I'll update that and add a CHANGELOG entry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

Copy link
Contributor

@MacMeDan MacMeDan left a comment

Choose a reason for hiding this comment

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

Thanks for the PR this will give users of the library better control if they want.

@MacMeDan MacMeDan self-assigned this Feb 23, 2018
@SD10
Copy link
Member

SD10 commented Feb 24, 2018

Thank you for reviewing this @MacMeDan ❤️ We aren't using the enhancement labels on PRs FYI

@SD10 SD10 removed the enhancement label Feb 24, 2018
@SD10 SD10 merged commit da5479e into MessageKit:master Feb 24, 2018
@SD10
Copy link
Member

SD10 commented Feb 24, 2018

Thank you for contributing to MessageKit! I've invited you to join the MessageKit GitHub organization - no pressure to accept! If you'd like more information on what that means, check out our contributing guidelines and join the MessageKit Slack channel. Feel free to reach out if you have any questions! 😃

@clayellis clayellis deleted the feature/auto-update-send branch February 24, 2018 15:29
@clayellis clayellis restored the feature/auto-update-send branch February 24, 2018 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants