Exposed doneButton enabled variable and added option to avoid auto-selection of day when releasing scrolling#112
Open
palla89 wants to merge 2 commits into
Open
Exposed doneButton enabled variable and added option to avoid auto-selection of day when releasing scrolling#112palla89 wants to merge 2 commits into
palla89 wants to merge 2 commits into
Conversation
itsmeichigo
requested changes
Dec 21, 2021
Owner
There was a problem hiding this comment.
Thank you @palla89 for the contribution! As mentioned in one of the comments below, I'm not fond of disabling auto-selection so I'd like to ask you to remove it from this PR if you don't mind. I also have a few comments on the done button update, please take a look 😄
Comment on lines
+17
to
+21
| public var doneButtonIsEnabled: Bool = true { | ||
| didSet { | ||
| doneButton.isEnabled = doneButtonIsEnabled | ||
| } | ||
| } |
Owner
There was a problem hiding this comment.
A few nits:
- I think it's nicer if we use a method instead of this long awkward variable name. So instead we can have something like:
func enableDoneButton(_ enabled: Bool). - The indenting doesn't quite match with the rest of the file, can you please update it?
- When the done button is disabled, I find that it still looks the same as when it's enabled so it doesn't look obvious and can be mistaken with an issue. How about updating the button's look (i.e make it grey) when it's disabled?
| } | ||
| } | ||
|
|
||
| public var autoSelectDayOnStopScroll: Bool = true |
Owner
There was a problem hiding this comment.
I'm not quite fond of this feature, as although someone requested it I find it looks buggy and not what I intended for the library. I think it's fine if someone wants it for their app, they can maybe tweak it to their need; but I'd prefer to leave it off the library for now. WDYT?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
var is
doneButtonIsEnabledand the other var isautoSelectDayOnStopScroll