On 3/22/07, Fred Kiefer <[EMAIL PROTECTED]> wrote:
Sergii Stoian wrote: > Hi, Fred > > On 3/22/07, Fred Kiefer <[EMAIL PROTECTED]> wrote: >> 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. > > That's why I post email before commit to SVN tree. > >> 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? > > There's no relation to other part of code. So please don't take it into > account. > But in general we need minimum height of NSBrowser list item. Is it wrong? > Not sure here. The minimal size should at least come from the selected font and not be hard coded. Perhaps using the defaultLineHeightForFont of the font. There are many places left in GNUstep, where we still use magic numbers, these should go away gradually.
Sure. I agree with you.
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? > > Ok. Let me explain why I made this change to code. I want to have NSBrowser > wich don't have to show first responder state to it's selected row in > last column. > I've try to achieve it by calling setShowsFirstResponder:NO to newly > created rows > which are NSMatrix cells. This is doesn't work with current SVN code. > The reason is: > NSMatrix drawCellAtRow: method set attribute cell to on calls NSCell > drawWithFrame:inView: > then set attribute to off again. So in this situation NSCell (which is > a part of NSMatrix) doesn't > have defined attribute. NSMatrix changes it dynamically whithout > respect to my code. > Do I understand this correctly, you want an NSBrowser where cells don't show their (selection or highlight) state by drawing a dotted frame? If this is what you want, why don't we implement the different NSFocusRingType values in NSCell and set the default type for NSBrowserCell to NSFocusRingTypeNone? I think the following code would do: // Draw first responder if (_cell.shows_first_responder && [[controlView window] firstResponder] == controlView) { switch (_cell.focus_ring_type) { case NSFocusRingTypeDefault: [[GSTheme theme] drawFocusFrame: [self drawingRectForBounds: cellFrame] view: controlView]; break; case NSFocusRingTypeExterior: [[GSTheme theme] drawFocusFrame: cellFrame view: controlView]; break; case NSFocusRingTypeNone: default: break; } } But neither this nor your patch would solve the problems I see when using an NSBrowser. Here highlighting of a cell may be switched of by selecting that cell once more. That way you cannot tell, which cell is active.
You're right. What you talking about (unhighliting of selected cell) is default behaviour of NSBrowser. By default NSBrowser shows responder ring of selected cell. So there's no problem here. Currently it is also not clear to me, which cells in the browser
get the focus ring drawn and which don't.
NSBrowser gets this information from NSMatrix (which is browser's column). And NSMatrix decides whether to draw focus ring or not. My feeling is that your patch hides the problems on NSBrowser and breaks
code in NSMatrix.
Yes, you're right. My change to NSMatrix was incorrect. Forget it please.
Later I understand why this algorithm was implemented: NSCell's > drawWithFrame:inView: > method draws dotted frame without checking of _cell.is_highlighted and > _cell.state attributes. > So why I make change to NSCell. > OK, but why do you think that highlight and state should have anything to do with the focus ring? >> Why is there no change to NSMatrix selectAll:? The other place where the >> first responder displaying of the cells gets changed. > > This is a good question. Main idea of this changes: do not show first > responder > state if I set it in my code explicitly (setShowsFirstResponder:NO). > No matter what selection were made multiple or single. How many cells with focus rings would you like to see in a matrix with multiple selected cells? None, one or all?
Current selecting is right. Just think about the fact that choice items are normally displayed in a
matrix. I think that your change worked for you as you only did it on NSCell, but not on NSButtonCell. > In general, I haven't check all code which have to adopt these changes. > I've posted these mail to check if I didn't missed something. > >> 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. > > Good point. Thanks. I'll do it. If nobody opposes, I will remove the focus ring drawing from these classes and update the code NSCell and NSButtonCell to take the focus ring style into account. Then you may experiment if setting the style to NSFocusRingTypeNone solves your problem. And perhaps somebody looks into the general issue as well.
I've attached fpatches to NSCell and NSMatrix as a beginning. The changes are: NSCell: [setShowsFirstResponder:]: update _cell.focus_ring_type wrt 'flag' parameter [drawWithFrame:inView:]: add proposed code by you. NSMatrix: method _drawCellAtRow:column: removed. It duplicates drawCellAtRow:column:. Some cleanups in drawCellAtRow:column. What do you think? -- Sergii Stoian, ProjectCenter maintainer
_______________________________________________ Discuss-gnustep mailing list [email protected] http://lists.gnu.org/mailman/listinfo/discuss-gnustep
