[etoys-dev] (SQ-179) Fix targeting in sliders, buttons and menus

[Bert Freudenberg]

On 10.08.2010, at 09:21, Jerome Peace (JIRA) wrote:
> 
> [ http://tracker.squeakland.org/browse/SQ-179? ] 
> 
> Jerome Peace commented on SQ-179:
> ---------------------------------
> 
> Okay. I loaded the change set. 

I installed the packages. Thanks for submitting to the inbox, it's easier for my workflow!

The added functionality is not needed for Etoys, but a welcome addition for anyone trying to go deeper. So I'm not opposed to let this in. I have a few complaints though, because added complexity is hard to take back, and we as a community will wind up having to maintain this code.

> Did some light testing. Target sighting worked for sliders. I could make a sliders for height: and width: an then change targets between two ellipses.

"set target" seems quite useless, as it unexpectedly refers to morphs under the slider itself.
"sight target" presents way too many objects. "potentialEmbeddingTargets" does a better job at filtering out unwanted objects. 

I'd only have a "set target" menu entry (and "clear target"), which should not be separated from the other entries ("set action selector" etc.).

> The change set dirtied four packages.
> 
> Graphics because of the target cursor. 

That cursor is bad. It does not have a white outline as all the other cursors, so it is invisible on dark objects. Why not use the crossHair cursor, which is what all other targetting uses? (Also, TargetCursor remained undeclared).

> Morph for the main bulk of the routines.

Shouldn't the new MenuMorph>>target: invoke updateItemsWithTarget:? Which MenuMorph>>setTarget: then could use?

The CustomMenu additions are not needed as far as I can tell.

It would be nice if the new menu entries had balloon help (see #balloonTextForLastItem:). That is no strict requirement but helps explain their purpose to documenters and other non-experts.

Also, the menus are not translatable yet. E.g. in targetWith:

	( self externalName, ' targets...' )

would properly be 

	('{1} targets...' translated format: {self externalName})

and given that this is a menu title and not an entry should be

	('Target for {1}' translated format: {self externalName})

Adding Collection>>asKnownNameMenu is too bad a style to accept. There is no precedent, and this should not become one. Rather do like you did in targetWith:.

And what is the change to #paintArea?

> Morphic Extras for two classes that needed those fixes.
> 
> Etoys for a one method patch to Button Properties. 
> 
> The updated packages are waiting in the inbox. They need to be loaded in the order above for best results.

Our current load order worked fine. To see it, open the "etoys" repository in MC, scroll down to "update" in the left-hand list, and click the newest update.mcm. In the bottom panel you can then see the load order.

> I only had to fix up one menu routine because of conflicts so everything should work smoothly. 
> 
> Or at least close enough for government work.
> 
> Anything else can be fixed within the time frame of the roadmap.
> 
> Problems from this point on will be minimal from what I can see.

Agreed, I think what I mentioned above looks simple to fix.

- Bert -