Alexander Wels has posted comments on this change.

Change subject: webadmin : CheckBoxGroup, DaysOfMonthSelector and DateTimeBox 
Widgets
......................................................................


Patch Set 12:

(6 comments)

A bunch of little comments, and one date format specific one. I think I gave 
you the right format for the default, but there are a ton of different ones if 
the one I gave you is not the one you are looking for.

https://gerrit.ovirt.org/#/c/37302/12/frontend/webadmin/modules/gwt-common/pom.xml
File frontend/webadmin/modules/gwt-common/pom.xml:

Line 72:       <version>${engine.version}</version>
Line 73:       <scope>provided</scope>
Line 74:     </dependency>
Line 75:     <dependency>
Line 76:          <groupId>org.gwtbootstrap3</groupId>
Replace tabs with spaces.
Line 77:          <artifactId>gwtbootstrap3-extras</artifactId>
Line 78:          <version>${gwtbootstrap3.version}</version>
Line 79:          <scope>provided</scope>
Line 80:     </dependency>


https://gerrit.ovirt.org/#/c/37302/12/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 19: import com.google.gwt.user.client.TakesValue;
Line 20: import com.google.gwt.user.client.ui.Widget;
Line 21: 
Line 22: public class GwtBootstrapDateTimePicker implements TakesValue<Date> {
Line 23:     public static final String DEFAULT_DATE_TIME_FORMAT = "mm/dd/yyyy 
hh:ii";//$NON-NLS-1$
ii seems like the wrong format, shouldn't the default be MM/dd/yyyy hh:mm?

Also I would probably go with something like this for the default format:

DateTimeFormat.getFormat(PredefinedFormat.DATE_TIME_SHORT).getPattern();

This will make the format Locale specific, instead of hard coding it to the US 
format.
Line 24:     private DateTimePicker dateTimePicker = new DateTimePicker();
Line 25:     private String dateTimeFormat;
Line 26: 
Line 27:     public GwtBootstrapDateTimePicker() {


https://gerrit.ovirt.org/#/c/37302/12/frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/ui/common/css/CheckBoxGroup.css
File 
frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/ui/common/css/CheckBoxGroup.css:

Line 2:         
spaces not tabs.


https://gerrit.ovirt.org/#/c/37302/12/frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/ui/common/css/DaysOfMonthSelector.css
File 
frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/ui/common/css/DaysOfMonthSelector.css:

Line 1: .selectedFlexTableCell {
Line 2:         background-color : #E6E6FA;
spaces not tabs
Line 3:         width: 25px;
Line 4: }
Line 5: 
Line 6: .normalFlexTableCell {


https://gerrit.ovirt.org/#/c/37302/12/frontend/webadmin/modules/userportal-gwtp/pom.xml
File frontend/webadmin/modules/userportal-gwtp/pom.xml:

Line 64:       <version>${gwtbootstrap3.version}</version>
Line 65:       <scope>provided</scope>
Line 66:     </dependency>
Line 67:     <dependency>
Line 68:          <groupId>org.gwtbootstrap3</groupId>
spaces not tabs
Line 69:          <artifactId>gwtbootstrap3-extras</artifactId>
Line 70:          <version>${gwtbootstrap3.version}</version>
Line 71:          <scope>provided</scope>
Line 72:     </dependency>


https://gerrit.ovirt.org/#/c/37302/12/frontend/webadmin/modules/webadmin/pom.xml
File frontend/webadmin/modules/webadmin/pom.xml:

Line 64:       <version>${gwtbootstrap3.version}</version>
Line 65:       <scope>provided</scope>
Line 66:     </dependency>
Line 67:     <dependency>
Line 68:          <groupId>org.gwtbootstrap3</groupId>
spaces not tabs
Line 69:          <artifactId>gwtbootstrap3-extras</artifactId>
Line 70:          <version>${gwtbootstrap3.version}</version>
Line 71:          <scope>provided</scope>
Line 72:     </dependency>


-- 
To view, visit https://gerrit.ovirt.org/37302
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I38daa0d2c151eb0e34603488496a8a1ea4719c87
Gerrit-PatchSet: 12
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: anmolbabu <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[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

Reply via email to