Issue 5474 in pharo: EventHandler Test + Better use of handler

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Issue 5474 in pharo: EventHandler Test + Better use of handler

pharo
Status: Accepted
Owner: [hidden email]
Labels: Type-Cleanup Milestone-1.4

New issue 5474 by [hidden email]: EventHandler Test + Better use of  
handler
http://code.google.com/p/pharo/issues/detail?id=5474

In most of the subclasses of morph, the methods invoking the handler are  
overriden and don't invoke the handler anymore.

I fix that but invoking the handler upstream.
I also create some tests for the handler (with the goal to rewrite a better  
one soon :) )

Slice is incoming


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5474 in pharo: EventHandler Test + Better use of handler

pharo
Updates:
        Status: FixReviewNeeded

Comment #1 on issue 5474 by [hidden email]: EventHandler Test +  
Better use of handler
http://code.google.com/p/pharo/issues/detail?id=5474

Slice is in the inbox


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5474 in pharo: EventHandler Test + Better use of handler

pharo
Updates:
        Cc: [hidden email]

Comment #2 on issue 5474 by [hidden email]: EventHandler Test +  
Better use of handler
http://code.google.com/p/pharo/issues/detail?id=5474

Guillermo, this fix breaks the current version of KeyMapping-Core (you  
override Morph>>#handlesKeystroke:)

I have seen that the latest version of KeyMapping-Core override another  
method (with no conflict this time :) ) but  this is not a stable version  
(I got a DNU about
     "self allEntries keymap" where allEntries return a dictionary
I didn't look in the code much deeper so I can't help more :)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5474 in pharo: EventHandler Test + Better use of handler

pharo

Comment #3 on issue 5474 by [hidden email]: EventHandler Test +  
Better use of handler
http://code.google.com/p/pharo/issues/detail?id=5474

The problem is that keymapping have to install the event handler on every  
morph instance with this change, and since it's an external package it will  
have to override every initialize(or similar) method on Morph classes :/.

The latest (but not stable) version aims to use Keydown instead of  
Keypress, but with that:
- That version is not compatible yet with it's predecessors.  If you load  
the bleeding edge on a clean image you'll not have the DNU
-shortcuts are handled twice (keymappings handle them on keydown and the  
original shortcuts handle them on keypress).

Maybe we can change the initialize on Morphic to something like

Morph>>initialize
        "initialize the state of the receiver"
        super initialize.
        owner := nil.
        submorphs := EmptyArray.
        bounds := self defaultBounds.
        color := self defaultColor
        self initializeKeymapping <-----"new"

Doing in that way the less harm...


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5474 in pharo: EventHandler Test + Better use of handler

pharo

Comment #4 on issue 5474 by [hidden email]: EventHandler Test +  
Better use of handler
http://code.google.com/p/pharo/issues/detail?id=5474

I do not understand why you mean by "The problem is that keymapping have to  
install the event handler on every morph instance with this change" ...

Right now, if you override the method (as you used to do) in the same way,  
it works.

I should miss something :s


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5474 in pharo: EventHandler Test + Better use of handler

pharo

Comment #5 on issue 5474 by [hidden email]: EventHandler Test +  
Better use of handler
http://code.google.com/p/pharo/issues/detail?id=5474

Yes, but if we remove that override to use EventHandler (That's what I  
understand you want to do :P), we have to install the event handlers :)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5474 in pharo: EventHandler Test + Better use of handler

pharo

Comment #6 on issue 5474 by [hidden email]: EventHandler Test +  
Better use of handler
http://code.google.com/p/pharo/issues/detail?id=5474

No, you can still override the method.
Just I wanted to tell you to prevent you spend 20 minutes to figure out why  
it doesn't work anymore :)

Just update you override in purpose, that's all :)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5474 in pharo: EventHandler Test + Better use of handler

pharo

Comment #7 on issue 5474 by [hidden email]: EventHandler Test +  
Better use of handler
http://code.google.com/p/pharo/issues/detail?id=5474

Is this for 1.4? if yes, is this good to integrate?


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5474 in pharo: EventHandler Test + Better use of handler

pharo

Comment #8 on issue 5474 by [hidden email]: EventHandler Test +  
Better use of handler
http://code.google.com/p/pharo/issues/detail?id=5474

Yes it is :)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5474 in pharo: EventHandler Test + Better use of handler

pharo
Updates:
        Status: FixToInclude

Comment #9 on issue 5474 by [hidden email]: EventHandler Test +  
Better use of handler
http://code.google.com/p/pharo/issues/detail?id=5474

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5474 in pharo: EventHandler Test + Better use of handler

pharo
Updates:
        Status: Integrated

Comment #10 on issue 5474 by [hidden email]: EventHandler Test +  
Better use of handler
http://code.google.com/p/pharo/issues/detail?id=5474

in 14411


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker