Skip to content

Loading…

Some TTSearchTextField fixs #502

Merged
merged 2 commits into from

3 participants

@vguerci

Just used TTSearchTextField for something else that a TTPickerTextField
Noticed two small bugs in it that I fixed, fixs that doesn't break the TTMessageController

@jwang

Thanks. Can you provide a test case so we can verify the issue and also the fix?

I'm trying to figure out why I can no longer tag pull requests.

@jverkoey

When would the search text field be presented within a scroll view? I assume that's why this commit changes it to use screenViewY instead of ttScreenY?

Actually, this commit raises a bigger question of why we have these utility methods in the first place when there are methods like

convert(Rect|Point):(from|to)(View|Window):
  1. I'm doing forms using tables mostly via TTTableControlItems, i had to do a kind of autocomplete field in the middle of the table... TTPostController doesn't have that issue because the TTPicker is the first row.

  2. Another TT mistery, maybe some pre-iOS2.0 code, ask joe :) My 2 cents is whatever convert methods implementation is, they can't be more optimized than these utilities, requiring traversal of UIViews tree. And also these are convenient methods, especially in scrollviews/tables where you only need Y, and also maybe more readable than convert...

1 - Aah, I see. This helps because I need some way to verify the fix. So it sounds like if I put a TTSearchTextField as something other than the first row it will recreate this bug, right?

2 - You're probably right about the implementations. I wonder if we could simply use the convert methods within these convenience methods instead of rewriting the functionality ourselves though.

  1. That's it, tried to reproduce it by altering TTMessageController w/o any result because of its implementation masking the issue.
    TTSearchTextField being almost an abstract control not used anywhere in TT except by TTPickerTextField subclass, it is hard to test without a custom subclass, like I the one I wrote :

    TTSTF

  2. I don't think there will be any real benefit doing this, plus the risk of breaking stuff... but imho, screenFrame is a better candidate for rewriting, in terms of performance and "cleanliness" (strange word, got a doubt there) :

    - (CGRect)screenFrame {
      return CGRectMake(self.screenViewX, self.screenViewY, self.width, self.height);
    }
    
@jverkoey

Cool. Looks good to me. Marking as reviewed.

@jwang jwang merged commit dcee090 into facebookarchive:development
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Showing with 2 additions and 2 deletions.
  1. +2 −2 src/Three20UI/Sources/TTSearchTextField.m
View
4 src/Three20UI/Sources/TTSearchTextField.m
@@ -268,7 +268,7 @@ - (void)tableView:(UITableView*)tableView didSelectRowAtIndexPath:(NSIndexPath*)
if ([_internal.delegate respondsToSelector:@selector(textField:didSelectObject:)]) {
id object = [_dataSource tableView:tableView objectForRowAtIndexPath:indexPath];
UITableViewCell* cell = [tableView cellForRowAtIndexPath:indexPath];
- if (cell.selectionStyle != UITableViewCellSeparatorStyleNone) {
+ if (cell.selectionStyle != UITableViewCellSelectionStyleNone) {
[_internal.delegate performSelector:@selector(textField:didSelectObject:) withObject:self
withObject:object];
}
@@ -480,7 +480,7 @@ - (CGRect)rectForSearchResults:(BOOL)withKeyboard {
CGFloat height = self.height;
CGFloat keyboardHeight = withKeyboard ? TTKeyboardHeight() : 0;
- CGFloat tableHeight = self.window.height - (self.ttScreenY + height + keyboardHeight);
+ CGFloat tableHeight = self.window.height - (self.screenViewY + height + keyboardHeight);
return CGRectMake(0, y + self.height-1, superview.frame.size.width, tableHeight+1);
}
Something went wrong with that request. Please try again.