Re: gui fixes

[Andreas Höschler]

Hi Fred,

> 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.

OK!

> > *
> > 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.

Let me know if you find a better one. :-)

> > 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?

The tableview is not made first responder in NSTableView mouseDown, at least my tableview gets first responder fine even with this patch applied. However, consider the case I mentioned in my other mail, an NSTextField with a formatter below the tableview with an invalid string. This textfield is firstResponder and it will resign to give up first responder since the string is invalid. Without this patch the user could click on any row in the tableview a change its selection. This must be prevented!

> > // 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.

The user enters a date into a texfield with a date formatter and presses return to trigger a fetch operation. A way (may be no nice one but anyway) to implement that in the controller class would be to make the controller delegate of the textfield and implement -control: textView: doCommandBySelector: to call fetch which access the textfield via an outlet, this gets the date and generates SQL...

Without my patch, [textfield objectValue] will return nil. :-( On MacOSX it does return the validated date.

> > 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.

if (validatedOK)
{
if (formatter) [_cell setObjectValue:newObjectValue];
          else [_cell setStringValue:newObjectValue];
}

Better?

Regards,

Andreas