Re: NSMatrix, NSCell, NSBrowser,NSBrowserCell proposed patch

[Fred Kiefer]

I think the general idea of this patch is good, but I don't understand
the details of it, which makes it hard for me to judge, if it is correct.

What is the relation of the change to NSBrowser to the rest of the
patch? I don't like hard-code numbers in the code. Why is this one
needed here?

If I remember the specification of NSMatrix correctly there are both the
concept of selected cells there as well as that of one dotted cell, the
later being the one with the first responder indicator. There may be
multiple selected cells, but only one dotted, that is key. cell. I am
not sure. if this concept is reflected in your code. You test if the
cell is highlighted and it's state is set. Does this match the above
conditions?

Why is there no change to NSMatrix selectAll:? The other place where the
first responder displaying of the cells gets changed.

The change to NSBrowserCell looks correct to me, but why not also remove
this code from NSImageCell and NSPopupButtonCell? And you will also need
to adopt the code in NSButtonCell to get buttons in a matrix working.

I am not opposing your change, but would like to have these questions
answered first.

Cheers,
Fred

Sergii Stoian wrote:
> Hi,
> 
> I want to propose changes to gui-trunk (patch attached). In general
> changes are touches NSMattrix and NSCell and is about drawing first
> reponder indicator (dotted frame). What is done:
> 
> - NSCell is responsible for showing first responder state;
> - Removed code from NSMatrix which sets attibute of cell to display
> first responder state then draws cell and then set attibute to off;
> - Removed code from NSBrowserCell which draws first responder indicator.
> 
> Before this patch (current SVN) NSCell draws first responder indicator
> when it's selected (highlighted) and when it's not slected. That's why
> NSMatrix and the like should switch on and off NSCell's attribute.
> 
> Please test the attached code. If there's no objections I'll commit
> this code to the trunk.
> 
> 
> ------------------------------------------------------------------------
> 
> Index: Source/NSBrowser.m
> ===================================================================
> --- Source/NSBrowser.m        (revision 24896)
> +++ Source/NSBrowser.m        (working copy)
> @@ -1842,6 +1842,7 @@
>         
>         cs = [sc contentSize];
>         ms = [matrix cellSize];
> +       if (ms.height < 15) ms.height = 15;
>         ms.width = cs.width;
>         [matrix setCellSize: ms];
>         [sc setDocumentView: matrix];
> Index: Source/NSMatrix.m
> ===================================================================
> --- Source/NSMatrix.m (revision 24896)
> +++ Source/NSMatrix.m (working copy)
> @@ -2086,21 +2086,7 @@
>         NSRectFill(cellFrame);
>       }
>        
> -      if (_dottedRow == row
> -          && _dottedColumn == column
> -       && [aCell acceptsFirstResponder]
> -          && [_window isKeyWindow]
> -       && [_window firstResponder] == self)
> -     {
> -       [aCell setShowsFirstResponder: YES];
> -          [aCell drawWithFrame: cellFrame inView: self];
> -          [aCell setShowsFirstResponder: NO];
> -     }
> -      else
> -     {
> -       [aCell setShowsFirstResponder: NO];
> -          [aCell drawWithFrame: cellFrame inView: self];
> -     }
> +      [aCell drawWithFrame: cellFrame inView: self];
>      }
>  }
>  
> @@ -4112,20 +4098,7 @@
>         NSRectFill(cellFrame);
>       }
>  
> -      if (_dottedRow == row && _dottedColumn == column
> -       && [aCell acceptsFirstResponder])
> -     {
> -       [aCell
> -         setShowsFirstResponder: ([_window isKeyWindow]
> -                                  && [_window firstResponder] == self)];
> -     }
> -      else
> -        {
> -       [aCell setShowsFirstResponder: NO];
> -     }
> -      
>        [aCell drawWithFrame: cellFrame inView: self];
> -      [aCell setShowsFirstResponder: NO];
>      }
>  }
>  
> Index: Source/NSCell.m
> ===================================================================
> --- Source/NSCell.m   (revision 24896)
> +++ Source/NSCell.m   (working copy)
> @@ -2017,7 +2017,8 @@
>  
>    // Draw first responder
>    if (_cell.shows_first_responder
> -    && [[controlView window] firstResponder] == controlView)
> +    && [[controlView window] firstResponder] == controlView
> +    && (_cell.is_highlighted || _cell.state))
>      {
>        // FIXME: Should depend on _cell.focus_ring_type
>        [[GSTheme theme] drawFocusFrame: [self drawingRectForBounds: 
> cellFrame] 
> Index: Source/NSBrowserCell.m
> ===================================================================
> --- Source/NSBrowserCell.m    (revision 24896)
> +++ Source/NSBrowserCell.m    (working copy)
> @@ -323,9 +323,6 @@
>    // Draw the body of the cell
>    [self _drawAttributedText: [self attributedStringValue]
>       inFrame: title_rect];
> -
> -  if (_cell.shows_first_responder == YES)
> -    NSDottedFrameRect(cellFrame);
>  }
>  
>  - (BOOL) isOpaque
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Discuss-gnustep mailing list
> Discuss-gnustep@gnu.org
> http://lists.gnu.org/mailman/listinfo/discuss-gnustep