> this and used a method off of a EventDispatchUtil class.
>>
>
> But we don't actually recycle events anywhere, do we? This is one of the
> open issues left from the review process as the event stuff was merged in to
> the 1.6 branch last year--why do we have this complicated mechanism that we
> don't actually use?
>
We recycle all dom events, so we are already in this use case.
It is not that we are pretending it is not public, the method is public, but
for a different set of users: Normal Widget/Event creators should never,
ever, use this method. However, with the decoupling of HandlerManager from
the rest of the event system we have introduced a new class of users
"handler management system implementors" They, and only they, should need
access to it.
>
>>
>>
>>
>> On Wed, Jan 14, 2009 at 4:36 PM, Ray Ryan <rj...@google.com> wrote:
>>
>>>
>>>
>>> On Wed, Jan 14, 2009 at 1:18 PM, Joel Webber <j...@google.com> wrote:
>>>
>>>> All,
>>>> I've run into a few snags trying to use the new event system with some
>>>> custom libraries, and would like to propose a few changes that would make
>>>> it
>>>> easier to reuse the GwtEvent hierarchy without necessarily using all of the
>>>> infrastructure in com.google.gwt.event.*
>>>>
>>>> When trying to fire GwtEvents without using HandlerManager (don't ask...
>>>> but it is a legitimate use-case), I hit a couple of brick walls. The first
>>>> was that GwtEvent.setSource() was package-protected, so that only
>>>> HandlerManager was intended to be able to call it. This makes it impossible
>>>> to usefully create one on your own.
>>>>
>>>> To solve this first problem, I simply added a static setEventSource()
>>>> method to HandlerManager. Even though it's in HandlerManager, I can use it
>>>> without pulling in the rest of it, and still not tempt event users to try
>>>> setting the event's source.
>>>>
>>>> public static void setEventSource(GwtEvent<?> event, Object source)
>>>> {
>>>>
>>>> event.setSource(source);
>>>>
>>>> }
>>>>
>>>
>>> I don't buy that this is an improvement over just making the method
>>> public. How do you figure that users will be less tempted to call this than
>>> to use the instance method?
>>>
>>> If it's that big a concern, let's make source an invariant, and make it a
>>> constructor argument on GwtEvent.
>>>
>>>>
>>>> The second issue was that HasHandlers had the method getHandlers() that
>>>> returns a HandlerManager. This effectively tightly couples HandlerManager
>>>> to
>>>> the use of GwtEvent. It also exposes all event sources' HandlerManagers as
>>>> part of their public API, which doesn't seem useful and constrains their
>>>> ability to evolve implementations in the future.
>>>>
>>>> My proposal for this problem is as follows:
>>>>
>>>> Create a HandlerCollection interface that HandlerManager
>>>> implements. This makes the use of HandlerManager an implementation detail
>>>> rather than an API decision.
>>>>
>>>>
>>>> public interface HandlerCollection {
>>>>
>>>> boolean isEventHandled(Type<?> type);
>>>>
>>>> void fireEvent(GwtEvent<?> event);
>>>>
>>>> }
>>>>
>>>> I then removed getHandlers() from HasHandlers (this interface is now
>>>> empty, and could probably be removed as well).
>>>>
>>>
>>> Can you go ahead and remove it?
>>>
>>>
>>>> This method was being used by methods such as ResizeEvent.fire(). I
>>>> propose simply requiring fire() to take the HandlerCollection as an
>>>> argument
>>>> (DomEvent.fire() already works this way, so it doesn't seem like much of a
>>>> stretch). This keeps the fire() methods from having to call
>>>> source.getHandlers(), and allows us to remove HasHandlers.getHandlers()
>>>> altogether.
>>>>
>>>> public static void fire(HandlerCollection handlers, HasResizeHandlers
>>>> source,
>>>>
>>>> int width, int height) { ... }
>>>>
>>>>
>>>> I've attached a patch that implements this change. It touches a lot of
>>>> files, but the changes are all pretty simple. I also refactored the Event
>>>> modules slightly, so that it's possible to include only the shared event
>>>> infrastructure, and just the logical and/or dom events (I also fixed the
>>>> fact that the dom event module didn't have an explicit inherits of
>>>> User.gwt.xml (for the Event class -- mostly a technicality since it will
>>>> always be included practically speaking).
>>>>
>>>
>>> If you upload the patch to Rietveld, I'll be happy to take the review.
>>>
>>>>
>>>> One other quick question: Does Widget.isEventHandled() actually get used
>>>> anywhere? I can't find any references to it. I can see it perhaps being
>>>> useful for widget implementors, but so far no one does, and it's public,
>>>> which seems a bit odd.
>>>>
>>>
>>> Please remove it.
>>>
>>>>
>>>> The patch is not 100% tested yet, but I wanted to get feedback before
>>>> taking it any further. This may seem like a pedantic exercise, but I think
>>>> it's important to start taking steps to carefully decouple modules sooner
>>>> rather than later.
>>>>
>>>> Cheers,
>>>> joel.
>>>>
>>>>
>>>
>>
>>
>> --
>> "There are only 10 types of people in the world: Those who understand
>> binary, and those who don't"
>>
>
>
--
"There are only 10 types of people in the world: Those who understand
binary, and those who don't"
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
