Greg Sheremeta has posted comments on this change. Change subject: userportal, webadmin: convert all grids (Cells) to PatternFly tooltips ......................................................................
Patch Set 20: (21 comments) https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/AbstractCell.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/AbstractCell.java: Line 64: if (tooltipContent == null) { Line 65: tooltipContent = getTooltip(value); Line 66: } Line 67: Line 68: if (BrowserEvents.MOUSEOVER.equals(event.getType())) { > At this point, assuming that "tooltipContent" is still null (likely scenari Yes. A "null" tooltip is a valid value that means no tooltip. And it's possible for a rendered cell to change its tooltip dynamically. So if a Cell is changing from some tooltip to no tooltip, these methods must run. Line 69: TooltipMixin.configureTooltip(parent, tooltipContent, event); Line 70: } Line 71: Line 72: if (BrowserEvents.MOUSEOUT.equals(event.getType())) { https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/AbstractImageButtonCell.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/AbstractImageButtonCell.java: Line 32: } Line 33: Line 34: @Override Line 35: public Set<String> getConsumedEvents() { Line 36: Set<String> set = new HashSet<String>(); > Why not simply: Done Line 37: set.add(BrowserEvents.CLICK); Line 38: set.addAll(super.getConsumedEvents()); Line 39: return set; Line 40: } https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/AbstractInputCell.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/AbstractInputCell.java: Line 62: if (tooltipContent == null) { Line 63: tooltipContent = getTooltip(value); Line 64: } Line 65: Line 66: if (BrowserEvents.MOUSEOVER.equals(event.getType())) { > Same comment as in AbstractCell.java same response Line 67: TooltipMixin.configureTooltip(parent, tooltipContent, event); Line 68: } Line 69: Line 70: if (BrowserEvents.MOUSEOUT.equals(event.getType())) { https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/CompositeCell.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/CompositeCell.java: Line 65: if (tooltipContent == null) { Line 66: tooltipContent = getTooltip(value); Line 67: } Line 68: Line 69: if (BrowserEvents.MOUSEOVER.equals(event.getType())) { > Same comment as in AbstractCell.java same response Line 70: TooltipMixin.configureTooltip(parent, tooltipContent, event); Line 71: } Line 72: Line 73: if (BrowserEvents.MOUSEOUT.equals(event.getType())) { Line 135: sb.appendHtmlConstant("</span>"); //$NON-NLS-1$ Line 136: } Line 137: else { Line 138: throw new IllegalStateException("CompositeCell cannot render Cells that do not implement " //$NON-NLS-1$ Line 139: + "org.ovirt.engine.ui.common.widget.table.cell.Cell"); //$NON-NLS-1$ > AFAIK, you could use Class.getName() here to avoid hard-coding fully-qualif Done Line 140: } Line 141: } Line 142: Line 143: public void setElementIdPrefix(String elementIdPrefix) { https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/EditTextCell.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/EditTextCell.java: Line 21: private com.google.gwt.cell.client.EditTextCell delegate = new com.google.gwt.cell.client.EditTextCell(); Line 22: Line 23: @Override Line 24: public Set<String> getConsumedEvents() { Line 25: Set<String> set = new HashSet<String>(); > Same comment as in AbstractImageButtonCell.java Done Line 26: set.add(BrowserEvents.CLICK); Line 27: set.add(BrowserEvents.KEYUP); Line 28: set.add(BrowserEvents.KEYDOWN); Line 29: set.add(BrowserEvents.BLUR); https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/ImageButtonCell.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/ImageButtonCell.java: Line 53: * Events to sink. Line 54: */ Line 55: @Override Line 56: public Set<String> getConsumedEvents() { Line 57: Set<String> set = new HashSet<String>(); > Same comment as in AbstractImageButtonCell.java Done Line 58: set.addAll(super.getConsumedEvents()); Line 59: set.add(BrowserEvents.CLICK); Line 60: return set; Line 61: } https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/LinkCell.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/LinkCell.java: Line 23: public class LinkCell extends TextCell { Line 24: Line 25: @Override Line 26: public Set<String> getConsumedEvents() { Line 27: Set<String> set = new HashSet<String>(); > Same comment as in AbstractImageButtonCell.java Done Line 28: set.add(BrowserEvents.CLICK); Line 29: set.addAll(super.getConsumedEvents()); Line 30: return set; Line 31: } https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/ListModelListBoxCell.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/ListModelListBoxCell.java: Line 50: } Line 51: Line 52: @Override Line 53: public Set<String> getConsumedEvents() { Line 54: Set<String> set = new HashSet<String>(); > Same comment as in AbstractImageButtonCell.java Done Line 55: set.add(BrowserEvents.CHANGE); Line 56: set.addAll(super.getConsumedEvents()); Line 57: return set; Line 58: } https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/ResizableCell.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/ResizableCell.java: Line 11: */ Line 12: public class ResizableCell extends SafeHtmlCell { Line 13: @Override Line 14: public Set<String> getConsumedEvents() { Line 15: Set<String> set = new HashSet<String>(); > Same comment as in AbstractImageButtonCell.java Done Line 16: set.add(BrowserEvents.CLICK); Line 17: set.add(BrowserEvents.MOUSEDOWN); Line 18: set.add(BrowserEvents.MOUSEMOVE); Line 19: set.addAll(super.getConsumedEvents()); https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/ResizableCheckboxCell.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/ResizableCheckboxCell.java: Line 11: */ Line 12: public class ResizableCheckboxCell extends SafeHtmlCell { Line 13: @Override Line 14: public Set<String> getConsumedEvents() { Line 15: Set<String> set = new HashSet<String>(); > Same comment as in AbstractImageButtonCell.java Done Line 16: set.add(BrowserEvents.CLICK); Line 17: set.add(BrowserEvents.MOUSEDOWN); Line 18: set.add(BrowserEvents.MOUSEMOVE); Line 19: set.add(BrowserEvents.CHANGE); https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/SafeHtmlCell.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/SafeHtmlCell.java: Line 10: Line 11: @Override Line 12: public void render(Context context, SafeHtml value, SafeHtmlBuilder sb, String id) { Line 13: if (value != null) { Line 14: sb.appendHtmlConstant("<div id=\" " + id + "\" style='display:block'>"); //$NON-NLS-1$ //$NON-NLS-2$ > Why the added space? Done (bad rebase) Line 15: sb.append(value); Line 16: sb.appendHtmlConstant("</div>"); //$NON-NLS-1$ Line 17: } Line 18: } https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/StatusCompositeCellWithElementId.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/StatusCompositeCellWithElementId.java: Line 70: cell.render(context, hasCell.getValue(value), sb, id); Line 71: } Line 72: else { Line 73: throw new IllegalStateException("StatusCompositeCell cannot render Cells that do not implement " //$NON-NLS-1$ Line 74: + "org.ovirt.engine.ui.common.widget.table.cell.Cell"); //$NON-NLS-1$ > Same comment as in CompositeCell.java Done Line 75: } Line 76: } Line 77: Line 78: @Override https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/TextCell.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/TextCell.java: Line 18: * Line 19: * If truncation is enabled, and if the text doesn't fit in the parent element, it is truncated. Line 20: * Line 21: * There are two types of truncation. You can specify a length in characters, or if you don't, overflow Line 22: * will be detected and truncated via CSS 'text-overflow: ellipse'. > ellipsis :-) Done Line 23: * Line 24: * Truncation can also be disabled. Line 25: * Line 26: * When text is truncated, the full text will be rendered in a tooltip, unless a different tooltip is manually https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/column/DiskContainersColumn.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/column/DiskContainersColumn.java: Line 35: } Line 36: Line 37: @Override Line 38: public SafeHtml getTooltip(Disk object) { Line 39: if (object.getNumberOfVms() < 2) { > Is this condition consistent with original behavior? Yes. If there is one VM, that VM's name is rendered. If there are two VMs, the string "2 VMs" is rendered, and the tooltip value should then list those VM names. Line 40: return null; Line 41: } Line 42: return SafeHtmlUtils.fromString(StringUtils.join(object.getVmNames(), ", ")); //$NON-NLS-1$ Line 43: } https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/header/ResizableHeader.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/header/ResizableHeader.java: Line 115: this(text, column, table, Line 116: new SafeHtmlCell() { Line 117: @Override Line 118: public Set<String> getConsumedEvents() { Line 119: Set<String> set = new HashSet<String>(); > Same comment as in AbstractImageButtonCell.java This class is changed to an interface in latest patchset. Line 120: set.add(BrowserEvents.CLICK); Line 121: set.addAll(super.getConsumedEvents()); Line 122: return set; Line 123: } https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/header/ResizeableCheckboxHeader.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/header/ResizeableCheckboxHeader.java: Line 22: super(checkboxHeader.getTitle(), column, table, Line 23: new SafeHtmlCell() { Line 24: @Override Line 25: public Set<String> getConsumedEvents() { Line 26: Set<String> set = new HashSet<String>(); > Same comment as in AbstractImageButtonCell.java Done Line 27: set.add(BrowserEvents.CLICK); Line 28: set.add(BrowserEvents.CHANGE); Line 29: set.add(BrowserEvents.KEYDOWN); Line 30: set.addAll(super.getConsumedEvents()); https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/widget/extended/vm/AbstractConsoleButtonCell.java File frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/widget/extended/vm/AbstractConsoleButtonCell.java: Line 35: @Template("<div id=\"{0}\" class=\"{1}\" data-class=\"consoleButton\" />") Line 36: SafeHtml consoleButton(String id, String className); Line 37: } Line 38: Line 39: private final CellTemplate template = GWT.create(CellTemplate.class); > Could be static & initialized on first use (but we can do that later). As we've agreed, cell templates will all move to their own giant file :) Line 40: Line 41: private final ConsoleButtonCommand command; Line 42: Line 43: private final String enabledCss; Line 55: * Events to sink. Line 56: */ Line 57: @Override Line 58: public Set<String> getConsumedEvents() { Line 59: Set<String> set = new HashSet<String>(); > Same comment as in AbstractImageButtonCell.java Done Line 60: set.addAll(super.getConsumedEvents()); Line 61: set.add(BrowserEvents.CLICK); Line 62: return set; Line 63: } https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/cell/CustomSelectionCell.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/cell/CustomSelectionCell.java: Line 63: * to additional events. Line 64: */ Line 65: @Override Line 66: public Set<String> getConsumedEvents() { Line 67: Set<String> set = new HashSet<String>(); > Same comment as in AbstractImageButtonCell.java Done Line 68: set.add(BrowserEvents.CHANGE); Line 69: set.addAll(super.getConsumedEvents()); Line 70: return set; Line 71: } https://gerrit.ovirt.org/#/c/37935/20/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/cell/MenuCell.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/cell/MenuCell.java: Line 38: } Line 39: Line 40: @Override Line 41: public Set<String> getConsumedEvents() { Line 42: Set<String> set = new HashSet<String>(); > Same comment as in AbstractImageButtonCell.java Done Line 43: set.add(BrowserEvents.CLICK); Line 44: set.addAll(super.getConsumedEvents()); Line 45: return set; Line 46: } -- To view, visit https://gerrit.ovirt.org/37935 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I54820570f3dd9cd8306ba63a263c480b33eeb65d Gerrit-PatchSet: 20 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Martin Betak <mbe...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
