Vojtech Szocs has posted comments on this change. Change subject: userportal, webadmin: convert all grids (Cells) to PatternFly tooltips ......................................................................
Patch Set 20: (23 comments) https://gerrit.ovirt.org/#/c/37935/20//COMMIT_MSG Commit Message: Line 17: Line 18: The Cell infrastructure has been slightly reworked. Highlights: Line 19: Line 20: * All custom Cells that simply provided some variation of tooltip behavior were removed. Line 21: Tooltips are now provided for free by the root objects in the Cell and Column hierarchy. +1 Line 22: * Various cells/columns that simply rendered an image with a tooltip were replaced by Line 23: AbstractImageResourceColumn and ImageResourceCell. Line 24: * Almost all Columns use the template-method pattern for rendering a tooltip. Simply Line 25: override the getTooltip method with tooltip contents. The default implementation 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 scenario?) - does it still make sense to proceed with TooltipMixin stuff? 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: Set<String> set = new HashSet<>(super.getConsumedEvents()); set.add(BrowserEvents.CLICK); return set; ? 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 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 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-qualified class name: "CompositeCell cannot render Cells that do not implement " + Cell.getName() This is because GWT supports using (some) Java class metadata at runtime: http://www.gwtproject.org/doc/latest/RefJreEmulation.html#Package_java_lang 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 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 22: * Cell that renders ActionButtonDefinition-like image buttons. Supports tooltips. Line 23: * Line 24: * @param <T> Line 25: * The data type of the cell (the model) Line 26: * TODO rename abstract? It's possible, but developers can see it's "abstract" in their IDE anyway :-) (Personally I'm not a big fan of Abstract* names for all abstract Java classes, the name should be meaningful enough I guess.) Line 27: */ Line 28: public abstract class ImageButtonCell<T> extends AbstractCell<T> { Line 29: Line 30: interface CellTemplate extends SafeHtmlTemplates { 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 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 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 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 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 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? 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 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 :-) (I'm really nit-picky today, sorry.) 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? (I mean, original code didn't care about number of items in "object.getVmNames()" collection.) 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 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 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). 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 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 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 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