Yep, that's a requirement, the only question, in my mind, is how to name the modules.
On Thu, Jan 15, 2009 at 2:09 PM, Ray Cromwell <cromwell...@gmail.com> wrote: > > Being able to pick out only base events is crucial for me, as trying to > package the DOM classes in some environments leads to breakage (e.g. > Android)-Ray > > > > On Thu, Jan 15, 2009 at 10:45 AM, <j...@google.com> wrote: > >> >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/6 >> File user/src/com/google/gwt/event/Event.gwt.xml (right): >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/6#newcode2 >> Line 2: <source path="shared"/> >> On 2009/01/15 14:53:37, ecc wrote: >> > In almost all other cases in gwt, the module corresponding to the >> package name >> > included everything. What about renaming this module to >> EventInfrastructure or >> > EventBase if that is no longer true? >> >> The way things were before, it was impossible to take only the Event >> module, without picking up both the dom and logical packages. That would >> make this entire exercise pointless, because you wouldn't be able to >> reuse common event infrastructure without picking up a dependency on >> User. >> >> The only solutions to this are (a) what we have here or (b) to get rid >> of the event package hierarchy, which would leave us with something >> like: >> >> gwt.event.Event >> gwt.domevent.DomEvent >> gwt.logicalevent.LogicalEvent >> >> I don't care a great deal, but having a structure that makes it >> impossible to pick out the modules you want is unacceptable. >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/8 >> File user/src/com/google/gwt/event/shared/HandlerCollection.java >> (right): >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/8#newcode23 >> Line 23: */ >> On 2009/01/15 14:53:37, ecc wrote: >> > wording seems slightly awkward, can you rephrase? Something like >> "Contains a >> > collection of handlers, exposing the ability ... >> >> It doesn't actually *contain* a collection of handlers, but I've >> reworded it a bit anyway. >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/7 >> File user/src/com/google/gwt/event/shared/HandlerManager.java (right): >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/7#newcode307 >> Line 307: >> On 2009/01/15 14:53:37, ecc wrote: >> > would prefer to have this method on a separate util class that is then >> javadoc'd >> > to be only used by people implementing new handler collections >> > (EventManagementUtil maybe?) >> > >> >> Done. >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/7#newcode365 >> Line 365: HandlerManager.this.removeHandler((Type<EventHandler>) type, >> handler); >> On 2009/01/14 22:50:06, rjrjr wrote: >> > Why is this an improvement over DefaultHandlerRegistration? We have >> arguably >> > less type safety, and no fewer classes. >> >> Not sure I follow -- DefaultHandlerRegistration assumed and required the >> use of HandlerManager, so you couldn't use them separately. All it >> appeared to do was implement the semantics of an inner class with a lot >> more typing (i.e., it had manually written ctor and fields that are just >> copies of now-final locals (HandlerManager.this, type, handler), and >> both implementations of removeHandler() contain the exact same typecast. >> I can't figure out a single reason for separating it into a top-level >> class. >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/24 >> File user/src/com/google/gwt/user/client/ui/Widget.java (left): >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/24#oldcode53 >> Line 53: public final HandlerManager getHandlers() { >> On 2009/01/15 15:01:08, rjrjr wrote: >> > On 2009/01/15 14:53:37, ecc wrote: >> > > I think that would be a much larger breaking change if we did not >> override the >> > > return type here. >> > >> > I think it's too soon to think that way with this code. All of its >> users are >> > early adopters who haven't even had access to it in an RC, or internal >> users >> > whom we're perfectly capable of fixing. >> > >> > Narrowing the type is the odd thing to do, and if we don't have a >> compelling >> > reason to do it, we shouldn't. IMHO >> >> I'm with Ray on this one -- I originally had tightened it down to >> HandlerManager because it wasn't an implementation of an interface >> method. But there's no need for the covariance here, because widgets >> never use it directly. I've loosened the type back to HandlerCollection, >> which feels a lot cleaner (I had to add one typecast to ListenerWrapper, >> but that's a temporary, deprecated glue class, so that feels ok to me). >> >> BTW, I dropped the javadoc because it's a trivial implementation of an >> interface method. There's no reason to specialize the documentation, and >> it will get picked up from the interface anyway. >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/21 >> File user/src/com/google/gwt/user/datepicker/client/DateBox.java >> (right): >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/21#newcode443 >> Line 443: DateChangeEvent.fireIfNotEqualDates(getHandlers(), this, >> oldDate, date); >> On 2009/01/15 14:53:37, ecc wrote: >> > I think this was a remnant from the previous patch. >> >> Thanks. Removed. >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/20 >> File user/src/com/google/gwt/user/datepicker/client/DateChangeEvent.java >> (right): >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/20#newcode42 >> Line 42: HandlerCollection handlers, S source, Date oldValue, Date >> newValue) { >> On 2009/01/15 14:53:37, ecc wrote: >> > I think this is a remnant from the previous patch. >> >> Yup. Removed. >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/3 >> File user/test/com/google/gwt/user/client/WindowTest.java (right): >> >> http://gwt-code-reviews.appspot.com/2205/diff/1/3#newcode191 >> Line 191: ResizeEvent.fire(Window.handlers, Window.handlers, 0, 0); >> On 2009/01/14 22:50:06, rjrjr wrote: >> > this won't compile... >> >> Whoops, diff update garbage. Removed. >> >> http://gwt-code-reviews.appspot.com/2205 >> >> >> >> > -- "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 -~----------~----~----~----~------~----~------~--~---
