Suggestions #17
Open
Suggestions #17
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
added 10 commits
August 11, 2014 10:55
Thus far, we have the concept of autocomplete, the data source API, and callbacks where we would be showing and hiding the table view so far. Definitely more work to complete it, but this is a good checkpoint.
This is still a work in progress, but the table view will actually render in the appropriate location now. At some point I appear to have broken the VENTokenField resizing, and actually taking an auto complete suggestion is still not supported. Just another checkpoint in the implementation.
By adding the table to the active window rather than to the tokenField’s super view, I’m avoiding a bug where the frame of the token field is reset when it should be growing. This should also help make sure the table view is on top of any other views that happen to exist in the super view.
I made the decision that autocomplete is just a typing accelerator, meaning that there is no dedicated callback for selecting an autocompleted value, instead the actual value selected is sent back just as if it was typed manually. I may change this before everything is complete, as this pushes more (potential) verification on the client without a lot saved for us.
Instead of having an API that involves handing over a full array (that may be thousands of items long) I switched over to one more similar to a table view. I also broke out all of the autocomplete stuff to a separate protocol instead of expanding the current data source.
Rather than making the client distinguish whether the entered text came from a suggestion or not, we now have a delegate method for having selected a suggestion. This is fired in addition to tokenField:didEnterText: in order to allow users to opt in or to simply enable autocomplete and use the same delegate methods as before.
autocomplete has a bad connotation, and isn’t really what we’re looking for in this case. This is really just a suggestion engine.
Xcode dropped them in the wrong folder and I didn’t notice that until now. Fixing it…
|
I found a bit of a bug with the table view placement when I started integrating this into my project that I need to get fixed, so please don't merge again for a bit. |
added 2 commits
August 12, 2014 15:05
XCTest runners test in what appears to be alphabetical order, or something close to it. Because of the naming, this test runs after resignFirstResponder, which leaves the system in a state where that test’s preconditions aren’t met - the keyboard is not up and ready to input in the text field. By changing from assuming the first responder to entering into the known good field, we stop worrying about order. I also applied this fix to the testBasicFlow test, which had to be broken into two steps due to the token field clearing properties.
We need to translate the frame of the tokenView into our destination view’s(the window) coordinates to keep the view properly aligned. Luckily, convertRect:toView: exists and does this hard work for us!
|
Looks like that last push fixed the issue, thanks convertRect:toView:! I've got this in the tableHeaderView of a UITableView under a nav bar, and everything is still lining up correctly with autocorrect. Everything looks good on my end again. |
|
After using this in reality, I'm realizing that the tokenField:didSelectSuggestion: should actually be updated to include the search string and index. This API forces the client to do some inefficient searching, whereas that can cut some time down. I'd like to update this before it gets merged - probably should have gone through some more integration before sending the pull request :disappointed_relieved: |
added 2 commits
August 13, 2014 14:44
in real use, tokenField:didSelectSuggestion: didn’t provide the necessary information to map back to a list of full data objects without a lot of inefficient searching. This adds some information back to allow easy indexing into an underlying data store to avoid this pitfall.
|
@bmnick Love this! Thanks for sharing. Would love to see this integrated. |
ayanonagon
force-pushed
the
master
branch
2 times, most recently
from
19b4b61
to
a0495aa
Compare
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 adds the ability to provide suggestions (like in the gif, actually) to become tokens. This is a fairly major addition, but it only requires a minor version bump, as it's exclusively adding new API. It does add a separate protocol, but that allows clients to use the token field as it used to exist easily and to separate suggestion logic (usually pretty different from the token logic).