Greg Sheremeta has posted comments on this change. Change subject: webadmin : gwt-bootstrap DateTimeBox widget ......................................................................
Patch Set 1: (4 comments) overall nice patch. thanks! https://gerrit.ovirt.org/#/c/38729/1/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/GwtCommon.gwt.xml File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/GwtCommon.gwt.xml: Line 11: < we definitely don't want to use GwtBootstrap3NoTheme -- I just submitted a patch to remove that and replace it with GwtBootstrap3CDN, so this might be a bad rebase -- or is it intentional? If it's intentional, what is it loading that you need? If this was a mistake, remove this line and see if everything still works. https://gerrit.ovirt.org/#/c/38729/1/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelDateTimeBox.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelDateTimeBox.java: Line 65: } Line 66: Line 67: @Override Line 68: public void setAcceptableValues(Collection<Date> values) { Line 69: } supposed to be emtpy? https://gerrit.ovirt.org/#/c/38729/1/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/GwtBootstrapDateTimePicker.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/GwtBootstrapDateTimePicker.java: Line 21: import com.google.gwt.user.client.ui.Widget; Line 22: Line 23: public class GwtBootstrapDateTimePicker implements TakesValue<Date> { Line 24: /* Line 25: * public static final String DEFAULT_DATE_TIME_FORMAT = do you want to remove this comment, or explain why it's here? Line 26: * DateTimeFormat.getFormat(PredefinedFormat.DATE_TIME_SHORT).getPattern(); Line 27: */ Line 28: public static final String DEFAULT_DATE_TIME_FORMAT = "mm/dd/yyyy HH:ii";//$NON-NLS-1$ Line 29: public static final String DEFAULT_TIME_FORMAT = "HH:ii";//$NON-NLS-1$ Line 27: */ Line 28: public static final String DEFAULT_DATE_TIME_FORMAT = "mm/dd/yyyy HH:ii";//$NON-NLS-1$ Line 29: public static final String DEFAULT_TIME_FORMAT = "HH:ii";//$NON-NLS-1$ Line 30: public static final String DEFAULT_DATE_FORMAT = "mm/dd/yyyy";//$NON-NLS-1$ Line 31: private final DateTimePicker dateTimePicker = new DateTimePicker(); what's the reason for using a delegate instead of extending DateTimePicker? I'm fine with the delegate -- just curious. Maybe we can document the reason with a comment. http://en.wikipedia.org/wiki/Delegation_pattern Line 32: private String dateTimeFormat; Line 33: Line 34: public GwtBootstrapDateTimePicker() { Line 35: this(DEFAULT_DATE_TIME_FORMAT, true); -- To view, visit https://gerrit.ovirt.org/38729 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I92f2a9eced817d8ed040901255983e5673990f1f Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: anmolbabu <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Greg Sheremeta <[email protected]> Gerrit-Reviewer: anmolbabu <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
