Vojtech Szocs has posted comments on this change.
Change subject: webadmin: main tab bar scrollable
......................................................................
Patch Set 1: Code-Review+1
(5 comments)
Overall the patch looks good so far, looking forward to the new scrollable tab
bar widget :-)
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/tab/ScrollableTabBar.java
Line 89: return minWidth;
Line 90: }
Line 91: private void recalculateScrollPanelMaxWidth() {
Line 92: int minWidgetBarWidth = Math.max(getMaxWidth(),
getWidgetMinWidthNeeded());
Line 93: widgetBar.getElement().getStyle().setProperty("width",
minWidgetBarWidth + 15, Unit.PX); //$NON-NLS-1$
Just wondering, why do we need +15 PX here?
Line 94:
scrollPanel.getElement().getStyle().setProperty(MAX_WIDTH_STYLE, getMaxWidth(),
Unit.PX);
Line 95: }
Line 96:
Line 97: private int getMaxWidth() {
Line 90: }
Line 91: private void recalculateScrollPanelMaxWidth() {
Line 92: int minWidgetBarWidth = Math.max(getMaxWidth(),
getWidgetMinWidthNeeded());
Line 93: widgetBar.getElement().getStyle().setProperty("width",
minWidgetBarWidth + 15, Unit.PX); //$NON-NLS-1$
Line 94:
scrollPanel.getElement().getStyle().setProperty(MAX_WIDTH_STYLE, getMaxWidth(),
Unit.PX);
Does getMaxWidth() calculation depend on widgetBar's DOM element (updated via
previous statement)? If not, we could cache getMaxWidth() value to avoid its
re-calculation.
Line 95: }
Line 96:
Line 97: private int getMaxWidth() {
Line 98: int leftArrowWidth = scrollLeftButton.isVisible() ?
scrollLeftButton.getWidget().getOffsetWidth() : 0;
Line 107: @Override
Line 108: public void execute() {
Line 109: recalculateWidgetBarMinWidth();
Line 110: recalculateScrollPanelMaxWidth();
Line 111: recalculateWidgetBarMinWidth();
Just wondering, why do we need to do recalculateWidgetBarMinWidth() again?
Line 112: }
Line 113: });
Line 114: }
Line 115:
Line 185: if (lastTab == null) {
Line 186: return;
Line 187: }
Line 188:
Line 189: int newLeft =
parsePosition(widgetBar.getElement().getStyle().getLeft()) + diff;
Can't we use DOM element's getOffsetLeft instead of reading CSS "left"
attribute?
Line 190: int rightOfLastTab = getRightPosition(lastTab);
Line 191:
Line 192: // Don't scroll for a positive newLeft
Line 193: if (newLeft <= 0) {
Line 219: }
Line 220: return result;
Line 221: }
Line 222:
Line 223: private int parsePosition(String positionString) {
Hm, so far I didn't found any built-in support for parsing CSS attribute values
in GWT :-/
(There is UiBinder's LengthAttributeParser but its API doesn't seem usable
outside UiBinder context.)
Anyway, shouldn't we also check here if positionString ends with "px" and
return 0 if it doesn't?
Line 224: int position = 0;
Line 225: int sign = -1;
Line 226: int i = 0;
Line 227: if (positionString.charAt(0) == '-') {
--
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: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[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
