isaced / ISEmojiView

Emoji Keyboard for iOS
MIT License
522 stars 119 forks source link

Created delegate to make the UITextField use case more convenient #2

Closed taptilegames closed 7 years ago

taptilegames commented 8 years ago

Sorry for all the TODOs, I'll probably work through them in time :-)

Header has ended up in a separate patch, not done a pull request before...

taptilegames commented 8 years ago

Header below. Hopefully next time the site won't confuse me :-)

//
//  EmojiBasicDelegate.h
//  CatchAppDemo
//
//  Created by Andrew Thomas on 4/6/16.
//  Copyright © 2016 catchapp. All rights reserved.
//

#import <UIKit/UIKit.h>
#import "ISEmojiView.h"

@interface ISEmojiBasicDelegate : NSObject<ISEmojiViewDelegate>
@property (nonatomic, weak) UITextField* field;
@end

@interface UITextField(Range)
@property(nonatomic, assign) NSRange selectedRange;;
@end

@interface ISEmojiView(Setup)
+(ISEmojiBasicDelegate*)addTo:(UITextField*)field;
@end
isaced commented 8 years ago

Thank you very much for your Pull Request. See what you mean. I guess it's about trying to add a default implementation for ISEmojiViewDelegate.Including didSelectEmoji, didPressDeleteButton method, But there are some problems, "field" control may be UITextField, there may be UITextView, so for a library, a good design should cover more scenarios, keep it simple and easy to use. So your Pull Request can not be merged into the main branch, but I will optimize these features and code updates in the coming days, thank you again. 👊🏻

taptilegames commented 8 years ago

Of course. I may well have more time to play with this too, but it is a starting point. It's frustrating of course that UITextField and UITextView have completely different delegates that serve almost exactly the same purpose. One solution might be to have -(ISEmojiBasicDelegate)addTo:(id)anything as a dispatch method, then having -(ISEmojiBasicDelegate)addToField:(UITextField*)field etc. There are a load of other omissions too - for example it assumes the textfield contains emoji groups of length 2 - which is problematic if the field has a combination of text and emojis. It also doesn't handle all the delegate methods yet, just the one I happened to need for my project.

isaced commented 8 years ago

Sorry for the late reply, for UITextField and UITextView different, my idea is to use generics, such as using a UIView to receive these two types of instances (field). then use -isKindOfClass: to distinguish the different operations performed.

taptilegames commented 8 years ago

I think I'd have described UIView as a common superclass rather than generics. It's not all that nice, because if you specify type UIView* for a method signature as an API developer, then the developer using it will reasonably try and pass in any view they fancy (UISearchView, UIWebView an HTML form?). Taking in id at least warns them to be on their guard

And yes, using -isKindOfClass: is implicit in using a dispatch method in the way I described :-) . That said, I believe best practice is to use respondsToSelector (I wouldn't though).

As a rule of thumb, whenever you use -isKindOfClass you should reconsider you should instead be subclassing. In your case, you could create a category for UITextField and UITextView. You would then call methods like -didTypeCharacter:(NSString*)emoji on this category and they would do the right thing. Although, the more I think about this, the more I think we're both approaching this the wrong way - we're trying to re-implement cocoa's keyboard handling - whereas what we really want is some kind of robot method.

Andrew Thomas Founder / Developer TapTile Mobile TapTile Games Ltd. (08535731) www.taptilemobile.com

On 8 April 2016 at 03:22, isaced notifications@github.com wrote:

Sorry for the late reply, for UITextField and UITextView different, my idea is to use generics, such as using a UIView to receive these two types of instances (field). then use -isKindOfClass: to distinguish the different operations performed.

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/isaced/ISEmojiView/pull/2#issuecomment-207179216