Vojtech Szocs has posted comments on this change.
Change subject: userportal, webadmin: ScrollableTabBar
......................................................................
Patch Set 10: Code-Review+2
(4 comments)
Patch looks good now.
Some minor cleanup can be done here or in a separate patch.
Overall, nice work!
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/view/ScrollableTabBarView.java
Line 215: * Calculate the minimum width needed to display all the tabs on
the bar. This works even if there are some
Line 216: * right floating tabs.
Line 217: */
Line 218: private void recalculateWidgetBarMinWidth() {
Line 219: // Add 1 for browsers that don't report width properly.
This comment can be moved to line "minWidth++;" below, I can do that before
pushing.
Line 220:
widgetBar.getElement().getStyle().setProperty(MIN_WIDTH_STYLE,
calculateWidgetMinWidthNeeded(), Unit.PX);
Line 221: }
Line 222:
Line 223: /**
Line 402: }
Line 403:
Line 404: @Override
Line 405: public void fireEvent(GwtEvent<?> event) {
Line 406: // TODO Auto-generated method stub
I think this method was overrided accidentally, I can remove it before pushing.
Line 407:
Line 408: }
Line 409:
Line 410: @Override
Line 408: }
Line 409:
Line 410: @Override
Line 411: public HandlerRegistration addLoadHandler(LoadHandler handler) {
Line 412: // TODO Auto-generated method stub
As discussed, load handler didn't work out so we use addAttachHandler instead,
this method override is not necessary, I can remove it before pushing.
Line 413: return null;
Line 414: }
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/tab/ModelBoundTab.java
Line 48: if ("IsAvailable".equals(pcArgs.propertyName)) {
//$NON-NLS-1$
Line 49: boolean isAvailable =
modelProvider.getModel().getIsAvailable();
Line 50: setAccessible(isAvailable);
Line 51: }
Line 52: TabAccessibleChangeEvent.fire(ModelBoundTab.this,
ModelBoundTab.this);
Actually, I was thinking of something like this:
@Override
public void setAccessible(boolean accessible) {
if (accessible != isAccessible()) {
TabAccessibleChangeEvent.fire(this, this);
}
super.setAccessible(accessible);
}
It's not critical, previous implementation is fine for now, can be improved
later on.
Line 53: }
Line 54: });
Line 55: }
Line 56:
--
To view, visit http://gerrit.ovirt.org/21716
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I63dddb3c0026ea3a5c13c3d18daebd02e13b1043
Gerrit-PatchSet: 10
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: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
