Re: gui fixes

[Fred Kiefer]

Andreas Höschler wrote:
> Hi all,
> 
> here is a collection of gui fixes I applied to my tree to make it behave
> like MacOSX. Let me know what you think and apply them to trunk if
> applicable. Thanks to Tim McIntos for his improval of my validateEditing
> implementation.
> 
> Regards,
> 
> Andreas
> 
> 
> 
> *NSWindow*
> This check on _firstResponder is really import otherwise
> makeFirstResponder returns NO if [window makeFirstResponder:nil] is
> called, which is wrong.
> 
> - (BOOL) makeFirstResponder: (NSResponder*)aResponder
> {
> ...
> 
> _firstResponder = aResponder;
> if (_firstResponder) // <--- add this check
> {
> if (![_firstResponder becomeFirstResponder])
> {
> _firstResponder = self;
> [_firstResponder becomeFirstResponder];
> return NO;
> }
> }
> ...
> }

I adopted this to follow the specification more closely. We need to set
self as first responder when aResponder is nil.

> *
> NSTableView*
> Changing selection within the tableview should not be allowed if it is
> not firstResponder or if a cell being edited contains an invalid string.
> 
> - (void)mouseDown:(NSEvent *)theEvent
> {
> ...
> 
> 
> /* Stop editing if any */
> if (_textObject != nil)
> {
> if (_editedCell != nil && [_editedCell isEntryAcceptable:[_textObject
> text]] == NO)
> {
> NSBeep();
> return;
> }
> [self validateEditing];
> [self abortEditing];
> }

I can see some reason for this change, but somehow this seems to be the
wrong place for it. Or we will need a similar change in plenty of places
in NSTableView.

> 
> if ([[self window] firstResponder] != self)
> {
> NSBeep();
> return;
> }
> 

Why this? How would a table view ever become the first responder if we
are not allowed to click on it?

> // Determine row and column which were clicked
> ...
> }
> 
> 
> Check whether delegate responds to
> control:didFailToFormatString:errorDescription: before sending message
> and and accept empty string.
> 
> - (void)validateEditing
> {
> if (_textObject)
> {
> NSFormatter *formatter = [_editedCell formatter];
> NSString *string = AUTORELEASE ([[_textObject text] copy]);
> id newObjectValue = string;
> BOOL validatedOK = YES;
> 
> if (formatter != nil)
> {
> NSString *error;
> 
> validatedOK = [formatter getObjectValue:&newObjectValue
> forString:string
> errorDescription:&error];
> NSLog(@"validatedOK %d string '%@' --> %@ %@", validatedOK, string,
> newObjectValue, NSStringFromClass([newObjectValue class]));
> 
> if (!validatedOK)
> {
> newObjectValue = nil;
> 
> if ([_delegate
> respondsToSelector:@selector(control:didFailToFormatString:errorDescription:)])
> 
> {
> validatedOK = [_delegate control:self didFailToFormatString:string
> errorDescription:error];
> }
> else if ([string isEqualToString:@""])
> {
> validatedOK = YES;
> }
> }
> }
> 
> if (validatedOK)
> {
> [_editedCell setObjectValue: newObjectValue];
> 
> 
> if (_dataSource_editable)
> {
> NSTableColumn *tb;
> 
> 
> tb = [_tableColumns objectAtIndex: _editedColumn];
> 
> 
> [self _setObjectValue: newObjectValue
> forTableColumn: tb
> row: _editedRow];
> }
> }
> }
> }
> 

OK

> *NSCell*
> Accept empty string.
> 
> - (BOOL) isEntryAcceptable: (NSString*)aString
> {
> if (_formatter != nil)
> {
> id newObjectValue;
> return ([aString isEqualToString:@""] ? YES : [_formatter
> getObjectValue:&newObjectValue forString:aString errorDescription: NULL]);
> }
> else
> {
> return YES;
> }
> }
> 

OK

> *NSTextField*
> This fix makes sure, that objectValue is valid, when the delegate method
> is called (which is the case on MacOSX).
> 
> - (BOOL)textView:(NSTextView *)textView doCommandBySelector:(SEL)command
> {
> if (sel_eq (command, @selector(insertNewline:)))
> {
> [self validateEditing];
> }
> if (_delegate && [_delegate respondsToSelector:
> @selector(control:textView:doCommandBySelector:)])
> {
> return [_delegate control:self textView:textView
> doCommandBySelector:command];
> }
> 
> return NO;
> }

I need more explanation for this change. It looks wrong to me.


> 
> Check whether delegate does implement didFailToFormatString: and accept
> empty string.
> - (void)validateEditing
> {
> if (_text_object)
> {
> NSFormatter *formatter = [_cell formatter];
> NSString *string = AUTORELEASE ([[_text_object text] copy]);
> id newObjectValue = string;
> BOOL validatedOK = YES;
> 
> if (formatter != nil)
> {
> NSString *error;
> 
> validatedOK = [formatter getObjectValue:&newObjectValue
> forString:string
> errorDescription:&error];
> 
> if (!validatedOK)
> {
> newObjectValue = nil;
> 
> if ([_delegate
> respondsToSelector:@selector(control:didFailToFormatString:errorDescription:)])
> 
> {
> validatedOK = [_delegate control:self didFailToFormatString:string
> errorDescription:error];
> }
> else if ([string isEqualToString:@""])
> {
> validatedOK = YES;
> }
> }
> }
> 
> if (validatedOK)
> {
> [_cell setObjectValue:newObjectValue];
> }
> }
> }
> 

This changes some calls from setStringValue: into setObjectValue:, I am
not sure this is what we want.


> *NSControl*
> The delegate of an NSControl should be able to access validated values
> with objectValue in -controlTextDidChange:(NSNotification *)obj (at
> least that's the behavior on MacOSX).
> 
> - (void)textDidChange: (NSNotification *)aNotification
> {
> NSMutableDictionary *dict;
> 
> // validate NSNumbers immediately so that objectValue returns reasonable
> values in controlTextDidChange:
> if ([_cell isEntryAcceptable:[[aNotification object] text]])
> {
> [self validateEditing];
> }
> 
> 
> ...
> }
> 

Here I would like to get some evidence that this is what Cocoa does. You
could for example place a break point in validateEditing and see from
where it gets called.