[gwt-contrib] Add clear support to Style (issue1720803)
Reviewers: , Description: Style does not have a getter, setter or clear function for the 'clear' CSS property. Please review this at http://gwt-code-reviews.appspot.com/1720803/ Affected files: user/src/com/google/gwt/dom/client/Style.java user/src/com/google/gwt/safecss/shared/SafeStylesBuilder.java user/src/com/google/gwt/safecss/shared/SafeStylesUtils.java user/test/com/google/gwt/dom/client/StyleTest.java user/test/com/google/gwt/safecss/shared/GwtSafeStylesUtilsTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add HasEnabled to FileUpload. (issue1614808)
LGTM http://gwt-code-reviews.appspot.com/1614808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Issue 7038: CompositeEditor and ListEditor optimizations (issue1664803)
On Sat, May 19, 2012 at 9:16 AM, Thomas Broyer wrote: > >> So I agree that is >> seems logical that a "live" view would only last until the next >> flush() or at most until the next setValue(). But this isn't >> well-documented in this class or anywhere else that I've found so far. > > To > me, https://developers.google.com/web-toolkit/doc/latest/DevGuideUiEditors is > clear enough. YMMV though. It does explain a lot of things but I'm missing the part where it explains the lifetime of the view returned by getEditors(). I'd expect that to be explained in the javadoc for getEditors itself. >> So maybe we want to do that now? I also wonder if, for consistency, we >> want to make sure the live view goes stale on *every* call to >> setValue(), which would make our optimization a bit trickier; we >> should probably create a new ListValueProvider and copy all the >> subeditors into it. > > > ListEditorWrapper could track this in detach() and from then on act as an > immutable list (throwing exceptions if you try to change it). Sure, that seems like a separate issue though. Here's my concern: if you call setValue() twice with the same list, this patch will make the live view returned by getEditors() last longer than before. So I think maybe we shouldn't reuse the ListEditorWrapper instance, just the Editors themselves. ListEditorWrapper looks cheap to construct, so we might as well be consistent and avoid changing behavior. On the other hand, maybe you're expecting that the caller *knows* that they're calling setValue() twice with the same List, so they expect the view to last longer as a result, sort of like if we were returning the same list again without wrapping it. If so, maybe this is a change in behavior to be closer to what the user expects? > If you ask me, for consistency, I'd > remove the null-check in ListEditor and let it fail for 'null' values, just > like it did originally. It might have been added by mistake, I don't know, > it was added as part of a bigger change and wasn't tracked/documented at > all, so who knows? I'm not sure what you mean. Do you mean that setValue() should just throw an exception for null lists? Or does it blow up later? Or do we want to avoid blowing up? > That's the kind of behavior I'd rather fight against: getValue just after > setValue should try to preserve the value (null vs. empty string) Yes, good point. Except that in this case, there is no getValue() method, since this isn't a LeafValueEditor and doesn't implement HasValue. It looks like all changes get sent to the backing list via flush()? If we call setValue() with a null list, I don't think flush() can do anything, because we don't have any reference to a backing model to update! Therefore, any changes made in the ListEditor cannot be saved? You'd have to use an OptionalFieldEditor if you want to save it. So it seems like at most we can display a read-only, empty list. There's no point in letting the user edit something they can't save. Or maybe I missed something - pretty likely, actually. - Brian -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: DateBox should use ScheduledCommand instead of Command (issue1695804)
LGTM Reposting at http://gwt-code-reviews.appspot.com/1719803 http://gwt-code-reviews.appspot.com/1695804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] DateBox should use ScheduledCommand instead of Command (issue1719803)
Reviewers: rdayal,
Description:
DateBox should use ScheduledCommand instead of Command
Resubmission of issue 1695804
Thanks Patrick!
Please review this at http://gwt-code-reviews.appspot.com/1719803/
Affected files:
M user/src/com/google/gwt/user/datepicker/client/DateBox.java
Index: user/src/com/google/gwt/user/datepicker/client/DateBox.java
===
--- user/src/com/google/gwt/user/datepicker/client/DateBox.java (revision
10982)
+++ user/src/com/google/gwt/user/datepicker/client/DateBox.java (working
copy)
@@ -17,6 +17,8 @@
package com.google.gwt.user.datepicker.client;
import com.google.gwt.core.client.GWT;
+import com.google.gwt.core.client.Scheduler;
+import com.google.gwt.core.client.Scheduler.ScheduledCommand;
import com.google.gwt.editor.client.IsEditor;
import com.google.gwt.editor.client.LeafValueEditor;
import com.google.gwt.editor.client.adapters.TakesValueEditor;
@@ -35,8 +37,6 @@
import com.google.gwt.event.logical.shared.ValueChangeHandler;
import com.google.gwt.event.shared.HandlerRegistration;
import com.google.gwt.i18n.client.DateTimeFormat;
-import com.google.gwt.user.client.Command;
-import com.google.gwt.user.client.DeferredCommand;
import com.google.gwt.user.client.ui.Composite;
import com.google.gwt.user.client.ui.HasValue;
import com.google.gwt.user.client.ui.PopupPanel;
@@ -490,7 +490,7 @@
private void preventDatePickerPopup() {
allowDPShow = false;
-DeferredCommand.addCommand(new Command() {
+Scheduler.get().scheduleDeferred(new ScheduledCommand() {
public void execute() {
allowDPShow = true;
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: SplitLayoutPanel should use ScheduledCommand instead of Command (issue1695803)
Submitted as r10957 http://gwt-code-reviews.appspot.com/1695803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: HorizontalSplitPanel should use ScheduledCommand instead of Command (issue1694803)
LGTM Reposting internally at: http://gwt-code-reviews.appspot.com/1718803 http://gwt-code-reviews.appspot.com/1694803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] HorizontalSplitPanel should use ScheduledCommand instead of Command (issue1718803)
Reviewers: rdayal,
Description:
HorizontalSplitPanel should use ScheduledCommand instead of Command
Resubmitting issue 1694803
Thanks Patrick!
Please review this at http://gwt-code-reviews.appspot.com/1718803/
Affected files:
M user/src/com/google/gwt/user/client/ui/HorizontalSplitPanel.java
Index: user/src/com/google/gwt/user/client/ui/HorizontalSplitPanel.java
===
--- user/src/com/google/gwt/user/client/ui/HorizontalSplitPanel.java
(revision 10982)
+++ user/src/com/google/gwt/user/client/ui/HorizontalSplitPanel.java
(working copy)
@@ -16,13 +16,13 @@
package com.google.gwt.user.client.ui;
import com.google.gwt.core.client.GWT;
+import com.google.gwt.core.client.Scheduler;
+import com.google.gwt.core.client.Scheduler.ScheduledCommand;
import com.google.gwt.i18n.client.LocaleInfo;
import com.google.gwt.resources.client.ClientBundle;
import com.google.gwt.resources.client.ImageResource;
import com.google.gwt.safehtml.shared.SafeHtmlBuilder;
-import com.google.gwt.user.client.Command;
import com.google.gwt.user.client.DOM;
-import com.google.gwt.user.client.DeferredCommand;
import com.google.gwt.user.client.Element;
import com.google.gwt.user.client.Timer;
@@ -301,8 +301,7 @@
// If one tries to set the width of the LEFT element to
// before layout completes, the RIGHT element will
// appear to be blanked out.
-
- DeferredCommand.addCommand(new Command() {
+ Scheduler.get().scheduleDeferred(new ScheduledCommand() {
public void execute() {
setWidth(panel.getElement(LEFT), "0px");
}
@@ -579,7 +578,7 @@
* possible.
*/
setSplitPosition(lastSplitPosition);
-DeferredCommand.addCommand(new Command() {
+Scheduler.get().scheduleDeferred(new ScheduledCommand() {
public void execute() {
setSplitPosition(lastSplitPosition);
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Resubmission of Issue 1686803 (issue1717803)
Reviewers: rdayal, Description: Resubmission of Issue 1686803 Add text-align support to Style Thanks Patrick! Please review this at http://gwt-code-reviews.appspot.com/1717803/ Affected files: M user/src/com/google/gwt/dom/client/Style.java M user/src/com/google/gwt/safecss/shared/SafeStylesBuilder.java M user/src/com/google/gwt/safecss/shared/SafeStylesUtils.java M user/src/com/google/gwt/user/client/ui/HorizontalSplitPanel.java M user/test/com/google/gwt/dom/client/StyleTest.java M user/test/com/google/gwt/safecss/shared/GwtSafeStylesUtilsTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Issue 7230: allow absolute paths in ui:style's src="" (issue1660804)
Reposting for internal resubmission at: http://gwt-code-reviews.appspot.com/1716803 http://gwt-code-reviews.appspot.com/1660804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Resubmitting code review 1660804 (issue1716803)
Reviewers: rdayal,
Description:
Resubmitting code review 1660804
Allow absolute paths in ui:style's src=""
Thanks Thomas!
Fixes issue: 7230
Review by: rda...@google.com
Please review this at http://gwt-code-reviews.appspot.com/1716803/
Affected files:
M user/src/com/google/gwt/uibinder/rebind/model/ImplicitCssResource.java
M user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.ui.xml
Index:
user/src/com/google/gwt/uibinder/rebind/model/ImplicitCssResource.java
===
--- user/src/com/google/gwt/uibinder/rebind/model/ImplicitCssResource.java
(revision 10982)
+++ user/src/com/google/gwt/uibinder/rebind/model/ImplicitCssResource.java
(working copy)
@@ -193,8 +193,16 @@
for (String s : sources) {
String resourcePath = path + '/' + s;
+ // Try to find the resource relative to the package.
URL found = classLoader.getResource(resourcePath);
- if (null == found) {
+ /*
+ * If we didn't find the resource relative to the package, assume it
+ * is absolute.
+ */
+ if (found == null) {
+found = classLoader.getResource(s);
+ }
+ if (found == null) {
logger.die("Unable to find resource: " + resourcePath);
}
urls.add(found);
Index: user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.ui.xml
===
--- user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.ui.xml
(revision 10982)
+++ user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.ui.xml
(working copy)
@@ -107,7 +107,7 @@
-+
type='com.google.gwt.uibinder.test.client.WidgetBasedUi.Style'>
.menuBar {
font-family: sans-serif;
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Removing uses of deprecated Tree code in GWT showcase sample and TreeExample. (issue1712804)
committed as r10991 http://gwt-code-reviews.appspot.com/1712804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Integer.TYPE, etc. (issue1710804)
I suppose if I tracked down the commit that set it up for 2.4, I could just change whatever it changed...I'll look in to that. It seems like this shouldn't be too hard, but after making a new gwt24_25 conf file and creating new reference jars from gwt 2.4, I'm getting this error: apicheck-nobuild: [java] Found 38 new resources [java] Found 38 new resources [java] Checking for compile errors [java][ERROR] Errors in '/mock/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java' [java] [ERROR] Line 34: The type SafeUriHostedModeUtils is already defined [java] [ERROR] Unable to build typeOracle for gwt24userApi I'm not sure where the "/mock/" part of that path comes from. Usually that's for fake/in-memory compilation units for unit tests. http://gwt-code-reviews.appspot.com/1710804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Added style getters to UiRenderers (issue1700803)
http://gwt-code-reviews.appspot.com/1700803/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
File user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
(right):
http://gwt-code-reviews.appspot.com/1700803/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode1367
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:1367: for
(ImplicitCssResource css : bundleClass.getCssMethods()) {
On 2012/05/23 01:51:11, rdayal wrote:
Nit: refactor into a helper method (as the code is identical to the
three lines
below).
Done.
http://gwt-code-reviews.appspot.com/1700803/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode1568
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:1568: *
{@code Element}.
On 2012/05/23 01:51:11, rdayal wrote:
Update this comment to reflect the new checks that you'd added.
Done.
http://gwt-code-reviews.appspot.com/1700803/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode1576
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:1576: if
(field == null || (!FieldWriterType.DEFAULT.equals(field.getFieldType())
On 2012/05/23 01:51:11, rdayal wrote:
Maybe wrap this small block of code into a helper method (for
readability)?
In a sense, this method is a helper method intending to make the caller
easier to read.
I am not sure how extracting this block will make it more readable. I'd
get a confusing method signature and name :P
void dieIfNoFieldNorFieldTypeDefaultNorFieldTypeGeneratedCss(FieldWriter
field, String getterName, String fieldName)
Also, is there a reason as to why you moved this higher up in the
validation
order?
Yes, this way the following if-checks get simpler (field != null and
types are either GENERATED_CSS or DEFAULT). It did not matter before
because there was only one one type of ui:field being considered.
http://gwt-code-reviews.appspot.com/1700803/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode1578
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:1578:
die("%s does not match a \"ui:field='%s'\" declaration in %s",
getterName, fieldName,
On 2012/05/23 01:51:11, rdayal wrote:
This error message does not seem to reflect the fact that there may be
a
mismatch in the type (as opposed to a failure to find a matching
getter name)
Done.
http://gwt-code-reviews.appspot.com/1700803/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode2221
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:2221: //
Esle the non-root elements are found by id
On 2012/05/23 01:51:11, rdayal wrote:
Spelling (Else).
Move this comment within the else block.
Done.
http://gwt-code-reviews.appspot.com/1700803/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode2225
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:2225: //
return (ElementSubclass) findUiField(parent);
On 2012/05/23 01:51:11, rdayal wrote:
Seems like this comment is inaccurate.
Done.
http://gwt-code-reviews.appspot.com/1700803/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode2230
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:2230: //
return (ElementSubclass) findPreviouslyRendered(parent);
On 2012/05/23 01:51:11, rdayal wrote:
Seems like this comment is inaccurate.
Done.
http://gwt-code-reviews.appspot.com/1700803/diff/1/user/test/com/google/gwt/uibinder/rebind/AbstractUiBinderWriterTest.java
File
user/test/com/google/gwt/uibinder/rebind/AbstractUiBinderWriterTest.java
(right):
http://gwt-code-reviews.appspot.com/1700803/diff/1/user/test/com/google/gwt/uibinder/rebind/AbstractUiBinderWriterTest.java#newcode105
user/test/com/google/gwt/uibinder/rebind/AbstractUiBinderWriterTest.java:105:
public static final MockJavaResource UI_STYLE = new
MockJavaResource("foo.UiStyle") {
On 2012/05/23 01:51:11, rdayal wrote:
Good man (adding test code). I like your style.
http://www.anyclip.com/movies/starsky-hutch/biker-bar-fight/
(start watching at 1:20)
Yup. Blow by blow, test by test.
http://gwt-code-reviews.appspot.com/1700803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Removing uses of deprecated Tree code in GWT showcase sample and TreeExample. (issue1712804)
http://gwt-code-reviews.appspot.com/1712804/diff/1/samples/showcase/src/com/google/gwt/sample/showcase/client/content/lists/CwTree.java
File
samples/showcase/src/com/google/gwt/sample/showcase/client/content/lists/CwTree.java
(right):
http://gwt-code-reviews.appspot.com/1712804/diff/1/samples/showcase/src/com/google/gwt/sample/showcase/client/content/lists/CwTree.java#newcode194
samples/showcase/src/com/google/gwt/sample/showcase/client/content/lists/CwTree.java:194:
item.addTextItem("");
Agreed. We should add some API to provide the children the first time
the node is opened. I created issue 7387 to track this.
http://gwt-code-reviews.appspot.com/1712804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing a Chrome animation bug where animations never stop running. The timestamp that webkitReq... (issue1702803)
committed as r10989 http://gwt-code-reviews.appspot.com/1702803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] HELP: Generated Javascript contains wrong prototype assignments
> > Can you apply this patch to a local GWT trunk build and see if it helps: > > http://gwt-code-reviews.appspot.com/1513803/ > Using trunk with your patch does not solve the issue. In the first split point the code _ = CustomPlace_2.prototype = CustomPlace_0.prototype = CustomPlace.prototype = new AbstractPlace; is replaced with defineSeed(2037, 2025, makeCastMap([Q$Place, Q$AbstractPlace, Q$IPlace, Q$CustomPlace]), CustomPlace_0, CustomPlace_2); and in the second split point there is no equivalent defineSeed() method call which contains CustomPlace_1. So I still see ClassCastExceptions. -- J. -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Addresses a couple of my concerns re. RF validation. (issue1577806)
Ping (sorry for the spam, these are important issues that we should really try to fix in 2.5, so I'm just making sure they're not forgotten) http://gwt-code-reviews.appspot.com/1577806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Generated EditorContext class names take actual parameterization into account. (issue1352806)
Ping http://gwt-code-reviews.appspot.com/1352806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix issue 6959. (issue1587803)
Ping http://gwt-code-reviews.appspot.com/1587803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
