LGTM

I don't think we should expose any of the implementation classes.


http://gwt-code-reviews.appspot.com/1296801/diff/1/4
File
user/src/com/google/gwt/canvas/dom/client/CanvasElementSupportDetectedMaybe.java
(right):

http://gwt-code-reviews.appspot.com/1296801/diff/1/4#newcode28
user/src/com/google/gwt/canvas/dom/client/CanvasElementSupportDetectedMaybe.java:28:
public class CanvasElementSupportDetectedMaybe extends
CanvasElementSupportDetector {
This impl should be package protected

http://gwt-code-reviews.appspot.com/1296801/diff/1/5
File
user/src/com/google/gwt/canvas/dom/client/CanvasElementSupportDetectedNo.java
(right):

http://gwt-code-reviews.appspot.com/1296801/diff/1/5#newcode28
user/src/com/google/gwt/canvas/dom/client/CanvasElementSupportDetectedNo.java:28:
public class CanvasElementSupportDetectedNo extends
CanvasElementSupportDetector {
This impl should be package protected

http://gwt-code-reviews.appspot.com/1296801/diff/1/6
File
user/src/com/google/gwt/canvas/dom/client/CanvasElementSupportDetector.java
(right):

http://gwt-code-reviews.appspot.com/1296801/diff/1/6#newcode29
user/src/com/google/gwt/canvas/dom/client/CanvasElementSupportDetector.java:29:
public class CanvasElementSupportDetector {
I think this should be package protected, and users should be forced to
use Canvas.isSupported().

http://gwt-code-reviews.appspot.com/1296801/diff/1/6#newcode46
user/src/com/google/gwt/canvas/dom/client/CanvasElementSupportDetector.java:46:
return !!$doc.createElement('canvas').getContext;
This runtime check essentially means that we create two canvas elements
every time we want to create one. Instead, can we have
CanvasElementSupportDetector return the canvas element if it is
supported?  That way, Canvas can call
CanvasElementSupportDetector.isSupported(), and use the returned Canvas
element as its root element.

http://gwt-code-reviews.appspot.com/1296801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to