Vojtech Szocs has posted comments on this change.
Change subject: webadmin: feedback tooltip
......................................................................
Patch Set 2:
Hm, I see that the discussion converged towards panel(HTML)-based tooltips.
The patch itself looks OK, however if we're aiming for consistent tooltip look
& feel, we should also think about consistent infra/widget support to achieve
it.
For example:
* LabelWithToolTip might use DecoratedPopupPanel (tooltip window) + HTML
(tooltip content)
* StyledImageResourceCell uses ElementAwareDecoratedPopupPanel (tooltip window)
+ Label (tooltip content)
* InfoIcon uses DecoratedPopupPanel (tooltip window) + HTML (tooltip content)
We should have one consistent DecoratedPopupPanel-based implementation used
across UI, for example:
// Following are custom DecoratedPopupPanel subclasses, we might want to
revisit them:
// * ElementAwareDecoratedPopupPanel
// * FeatureNotImplementedYetPopup
// * ItemInfoPopup
/**
* @param <S>
* Tooltip source, i.e. widget that controls appearance
(mouseover) or hiding (mouseout) of the tooltip
* panel.
*/
public class TooltipPanel<S extends Widget & HasMouseOverHandlers &
HasMouseOutHandlers>
extends DecoratedPopupPanel {
public TooltipPanel(boolean autoHide) {
super(autoHide);
getElement().getStyle().setZIndex(1);
}
public TooltipPanel() {
this(false);
}
public void applyTo(final S tooltipSource) {
tooltipSource.addMouseOverHandler(new MouseOverHandler() {
@Override
public void onMouseOver(MouseOverEvent event) {
TooltipPanel.this.onTooltipSourceMouseOver(event);
TooltipPanel.this.showRelativeTo(tooltipSource);
}
});
tooltipSource.addMouseOutHandler(new MouseOutHandler() {
@Override
public void onMouseOut(MouseOutEvent event) {
TooltipPanel.this.onTooltipSourceMouseOut(event);
TooltipPanel.this.hide(true);
}
});
}
protected void onTooltipSourceMouseOver(MouseOverEvent event) {
// No-op, override as necessary
}
protected void onTooltipSourceMouseOut(MouseOutEvent event) {
// No-op, override as necessary
}
}
In InfoIcon, we could replace DecoratedPopupPanel with our TooltipPanel -
instead of this:
private final DecoratedPopupPanel infoPanel = new DecoratedPopupPanel(true);
...
infoPanel.setWidget(new HTML(text));
infoPanel.getElement().getStyle().setZIndex(1);
addMouseOutHandler(new MouseOutHandler() { /* code that updates InfoIcon's
widget and shows tooltip panel */ });
addMouseOverHandler(new MouseOverHandler() { /* code that updates InfoIcon's
widget and hides tooltip panel });
we could do this:
private final TooltipPanel<InfoIcon> infoPanel;
...
infoPanel = new TooltipPanel<InfoIcon>(true) {
@Override
protected void onTooltipSourceMouseOver(MouseOverEvent event) {
InfoIcon.this.setWidget(infoImageHover); // custom hook to preserve
original InfoIcon behavior
}
@Override
protected void onTooltipSourceMouseOut(MouseOutEvent event) {
InfoIcon.this.setWidget(infoImage); // custom hook to preserve
original InfoIcon behavior
}
};
infoPanel.setWidget(new HTML(text));
infoPanel.applyTo(this);
For simple cases like in LabelWithToolTip:
private final TooltipPanel<LabelWithToolTip> tooltipPanel = new
TooltipPanel<LabelWithToolTip>();
...
tooltipPanel.setWidget(tooltip); // HTML tooltip
tooltipPanel.applyTo(this);
...
// in setTitle method override
tooltip.setHTML(title); // no need to have separate String title attribute
We could even have a method like this:
tooltipPanel.setText(text); // -> setWidget(new HTML(text));
What do you think guys?
--
To view, visit http://gerrit.ovirt.org/25703
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id17d0addf99645d2c8fb6f54ccafa0d4d688d177
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[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: No
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
