[squeak-dev] Re: PluggableListMorph>>mouseUp: sets index to 0

Jerome Peace peace_the_dreamer at yahoo.com
Fri Jun 13 06:00:35 UTC 2008


[squeak-dev] PluggableListMorph>>mouseUp: sets index to 0

Hi Ross,

You have found an integration bug. 

See Mantis there are a bunch of reports on why 
PluggableListMorph>>mouseUp: was changed.

http://bugs.squeak.org/view.php?id=1347
0001347: Doubleclick on list entries not working properly

see also 2452,0976, 5630.

PluggableListMorphPlus was written before the change
so it is still probably expecting the old behavior. 
Andreas's position was that PLMPlus was meant to work with Tweak
and problems would have to be handled if used for regular squeak.
You have apparently found one of those problems.

Please add a report to mantis detailing how to come across your bug.
An sunit test the fails until the problem is fixed would be appreciated.
 By the time you do all that you might be able to come up with a fix.
 That also gets appreciated.

Hth, 

Yours in curiosity and service, --Jerome Peace

***
>Ross Boylan ross at biostat.ucsf.edu 
>Fri Jun 13 01:12:32 UTC 2008 
>
>
>PluggableListMorph>>mouseUp:
>mouseUp: event 
>	"The mouse came up within the list; take appropriate action"
>
Specifically,
find out what row we were in.

yada, yada

if row is identical to the selected index 
then see if we are to deselect it.
if we are then make sure we are not already deselected 
and deselect.
else row and current selection are different.
change the selection.
note this may also cause a deselection
if cursor was over an empty row.(e.g. the last row)
	
>	| row mdr |
>	row := self rowAtLocation: event position.
>	event hand hasSubmorphs ifFalse: [
>		mdr := self mouseDownRow.
>		 self mouseDownRow: nil.
>		mdr ifNil: [^self]].
>	(self enabled and: [model okToChange])
>		ifFalse: [^ self].
>	"No change if model is locked or receiver disabled"
>	row == self selectionIndex
>		ifTrue: [(autoDeselect ifNil: [true]) ifTrue:[row == 0 ifFalse: [self
>changeModelSelection: 0 "!!!!"] ]]
>		ifFalse: [self changeModelSelection: row].
>	Cursor normal show
>
>I marked the spot with "!!!!".  This leads to the index setter being
>called with argument 0 on the underlying model, which produces an error.

I believe the logic to be correct.

This is autodeselecting if you don't want that behavior set autodeselect to false.
Also the underlying model should handle deselection gracefully.

If its any help, it took me a while to puzzle out the original logic of this and to come up with the simplification of it. 	The method has too many responsibilities.

>
>I could code around this, but is this supposed to be the way things
>work?

Yes AFAIK.

>The context for this was that I had clicked on a row that was already
>selected.  I *did* want the selection to be communicated back to the
>model; the row was selected by default, and the user click indicated it
>was the right selection.

Hmmm. Clicking would deselect when autodeselect is true. 
Worse, with autodeselt false no amount of clicking
 would send the selection to the model.
That is the right thing for models with toggling actions
 but wrong for your needs.
You would have to bring up the list unselected.

>The PluggableListMorph (actually, PluggableListMorphPlus, but that's not
>where the method is implemented) was built by ToolBuilder.
>
>Ross
***



      



More information about the Squeak-dev mailing list