Vojtech Szocs has posted comments on this change.

Change subject: userportal, webadmin: convert InfoIcons to PatternFly tooltips
......................................................................


Patch Set 8:

(3 comments)

https://gerrit.ovirt.org/#/c/37990/8//COMMIT_MSG
Commit Message:

Line 12: 
Line 13: Note: this infrastructure required a custom copy of gwtbootstrap3's 
Tooltip
Line 14: class. I will submit a patch to gwtbootstrap3 to incorporate this 
behavior.
Line 15: When that is release and we upgrade, the custom copy of Tooltip can be 
replaced.
Line 16: There is a TODO-GWT in the code for this.
Thanks!
Line 17: 
Line 18: Enforced italic HTML in InfoIcon, with an option to disable.
Line 19: 
Line 20: Bug-Url: https://bugzilla.redhat.com/1067318


https://gerrit.ovirt.org/#/c/37990/8/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/view/popup/ConsolePopupView.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/view/popup/ConsolePopupView.java:

Line 12: import org.ovirt.engine.ui.common.widget.WidgetWithInfo;
Line 13: import org.ovirt.engine.ui.common.widget.dialog.SimpleDialogPanel;
Line 14: import 
org.ovirt.engine.ui.common.widget.editor.generic.EntityModelRadioButtonEditor;
Line 15: import org.ovirt.engine.ui.common.widget.tooltip.TooltipConfig.Width;
Line 16: import org.ovirt.engine.ui.common.widget.tooltip.WidgetTooltip;
I think this import can be removed.
Line 17: import 
org.ovirt.engine.ui.common.widget.uicommon.popup.console.EntityModelValueCheckBoxEditor;
Line 18: import 
org.ovirt.engine.ui.common.widget.uicommon.popup.console.EntityModelValueCheckbox.ValueCheckboxRenderer;
Line 19: import org.ovirt.engine.ui.uicommonweb.models.ConsolePopupModel;
Line 20: import org.ovirt.engine.ui.uicommonweb.models.ConsoleProtocol;


https://gerrit.ovirt.org/#/c/37990/8/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/dialog/InfoIcon.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/dialog/InfoIcon.java:

Line 49:     }
Line 50: 
Line 51:     private void _setText(String text) {
Line 52:         if (useItalic) {
Line 53:             text = wrapItalic(text);
I'm not a big fan of modifying parameters passed into methods, so personally 
I'd prefer following instead:

 tooltip.setText(useItalic ? wrapItalic(text) : text);

What do you think?

Also, I'd suggest to rename "_setText" to "setTooltipText" or similar.
Line 54:         }
Line 55:         tooltip.setText(text);
Line 56:     }
Line 57: 


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

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