Added search bar component #69
Merged
Conversation
This file contains 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
ilyapuchka
force-pushed
the
search-bar-component
branch
from
93b72fd
to
d79c600
Compare
ilyapuchka
force-pushed
the
search-bar-component
branch
from
72ffa4c
to
3bb984e
Compare
mluisbrown
added
enhancement
ilyapuchka
changed the title
sergdort
reviewed
Nov 7, 2018
|
|
||
| let searchBar = UISearchBar() | ||
|
|
||
| var textDidChange: Optional<(UISearchBar, String) -> Void> = nil |
There was a problem hiding this comment.
Do we really need the UISearchBar there?
There was a problem hiding this comment.
I decided to follow the signature of the delegate method. also it makes it easier to do things like dismiss keyboard when cancel is pressed (for some reason is not a default behaviour) without jumping through the hoops. it can always be ignored if not needed.
sergdort
reviewed
Nov 7, 2018
| public init( | ||
| placeholder: String? = nil, | ||
| keyboardType: UIKeyboardType = .default, | ||
| didBeginEditing: Optional<(UISearchBar) -> Void> = nil, |
sergdort
reviewed
Nov 7, 2018
|
|
||
| extension Component.Search { | ||
| public final class StyleSheet: BaseViewStyleSheet<View> { | ||
| public struct SearchBar: StyleSheetProtocol { |
There was a problem hiding this comment.
This should be moved to the StyleSheets framework. And should be extended from ViewStyleSheet as UISearchBar is a subclass of UIView
sergdort
suggested changes
Nov 7, 2018
|
@sergdort I've addressed your comments |
sergdort
reviewed
Nov 8, 2018
sergdort
reviewed
Nov 8, 2018
sergdort
reviewed
Nov 8, 2018
| super.apply(to: element) | ||
| element.setTextInputBackgroundColor(color: backgroundColor ?? .clear, | ||
| height: height, | ||
| cornerRadius: cornerRadius) |
There was a problem hiding this comment.
Because on UISearchBar one can only set the background image, not a colour.
sergdort
approved these changes
Nov 8, 2018
|
@sergdort addressed 😉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR adds a simple component that contains
UISearchBar.Motivation
Sometimes it might be needed just to display a search bar in the cell without the luggage of
UISearchViewControllerwhich might be hard sometimes to tweak to fit particular UX, and something that requires less customisation thanUITextFieldto make it look likeUISearchBar.Alternatives considered
There is currently
TextInputcomponent in BentoKit but it kind of serves a different purpose and also requires quite some tweaking and style additions to make it fit this use case. AlsoUISearchBaris not a subclass ofUITextFieldand have some extra features like showing cancel button and search icon out of the box (still could be done withTextInputbut not without more changes to it).UISearchViewControlleralso does not fit here very well as it requiresnavigationItemand it can be tricky to make it work with DrawerKit used here (see screenshot)Example