Daniel Erez has posted comments on this change.
Change subject: userportal: redesign for resources tab
......................................................................
Patch Set 3: (18 inline comments)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationMessages.java
Line 77:
Line 78: @DefaultMessage("No {0} to display")
Line 79: String noItemsToDisplay(String items);
Line 80:
Line 81: @DefaultMessage("Exceeded {0}% / {1}vCPU")
Move these to 'userportal/../ApplicationMessages'
Line 82: String exceedingCpus(int percentage, int number);
Line 83:
Line 84: @DefaultMessage("Exceeded {0}% / {1}")
Line 85: String exceedingMem(int percentage, String mem);
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/main/view/tab/extended/SideTabExtendedResourceView.java
Line 152: EventBus eventBus, ClientStorage clientStorage,
Line 153: SubTableResources headerResources, ApplicationResources
resources, ApplicationConstants constants) {
Line 154:
Line 155: vmTable = new VmTable(modelProvider, headerResources,
resources, constants);
Line 156: vmTable.setVisible(true);
why isn't it true by default?
Line 157:
Line 158: SimpleRefreshManager refreshManager = new
SimpleRefreshManager(modelProvider, eventBus, clientStorage);
Line 159: refreshPanel = refreshManager.getRefreshPanel();
Line 160:
Line 173: initQuotaList(model);
Line 174: }
Line 175: });
Line 176:
Line 177: Window.addResizeHandler(new ResizeHandler() {
Consider extracting it to a new method.
Line 178: @Override
Line 179: public void onResize(ResizeEvent resizeEvent) {
Line 180:
vcpuExpanderContent.setHeight(infoBoxCpu.getOffsetHeight() - 150 + "px");
//$NON-NLS-1$
Line 181:
memoryExpanderContent.setHeight(infoBoxMemory.getOffsetHeight() - 150 + "px");
//$NON-NLS-1$
Line 176:
Line 177: Window.addResizeHandler(new ResizeHandler() {
Line 178: @Override
Line 179: public void onResize(ResizeEvent resizeEvent) {
Line 180:
vcpuExpanderContent.setHeight(infoBoxCpu.getOffsetHeight() - 150 + "px");
//$NON-NLS-1$
Use constant (preferably with meaningful name...)
Line 181:
memoryExpanderContent.setHeight(infoBoxMemory.getOffsetHeight() - 150 + "px");
//$NON-NLS-1$
Line 182: vmTable.setHeight(bottomLayoutPanel.getOffsetHeight()
- 350 + "px"); //$NON-NLS-1$
Line 183: }
Line 184: });
Line 177: Window.addResizeHandler(new ResizeHandler() {
Line 178: @Override
Line 179: public void onResize(ResizeEvent resizeEvent) {
Line 180:
vcpuExpanderContent.setHeight(infoBoxCpu.getOffsetHeight() - 150 + "px");
//$NON-NLS-1$
Line 181:
memoryExpanderContent.setHeight(infoBoxMemory.getOffsetHeight() - 150 + "px");
//$NON-NLS-1$
same here
Line 182: vmTable.setHeight(bottomLayoutPanel.getOffsetHeight()
- 350 + "px"); //$NON-NLS-1$
Line 183: }
Line 184: });
Line 185:
Line 178: @Override
Line 179: public void onResize(ResizeEvent resizeEvent) {
Line 180:
vcpuExpanderContent.setHeight(infoBoxCpu.getOffsetHeight() - 150 + "px");
//$NON-NLS-1$
Line 181:
memoryExpanderContent.setHeight(infoBoxMemory.getOffsetHeight() - 150 + "px");
//$NON-NLS-1$
Line 182: vmTable.setHeight(bottomLayoutPanel.getOffsetHeight()
- 350 + "px"); //$NON-NLS-1$
same here
Line 183: }
Line 184: });
Line 185:
Line 186:
vcpuExpander.initWithContent(vcpuExpanderContent.getElement());
Line 244: quotaPerUserUsageEntity.getVcpuUsageForUser());
Line 245: addQuotaRow(cpusQuotasList,
quotaPerUserUsageEntity.getQuotaName(), vcpuQuotaProgressBar);
Line 246: }
Line 247:
Line 248: private void addQuotaToMemoryQuotaList(QuotaUsagePerUser
quotaPerUserUsageEntity) {
addQuotaToMemoryQuotaList and addQuotaToStorageQuotaList seem quite similiar.
Can it be generalized easily?
Line 249: QuotaProgressBar memoryQuotaProgressBar = new
QuotaProgressBar(QuotaProgressBar.QuotaType.MEM);
Line 250:
memoryQuotaProgressBar.setValues(quotaPerUserUsageEntity.getMemoryLimit(),
Line 251: quotaPerUserUsageEntity.getMemoryTotalUsage() -
quotaPerUserUsageEntity.getMemoryUsageForUser(),
Line 252: quotaPerUserUsageEntity.getMemoryUsageForUser());
Line 270: flowPanel.add(progressBar);
Line 271: list.add(flowPanel);
Line 272: }
Line 273:
Line 274: private void aggregate(QuotaUsagePerUser aggregatedUsage,
QuotaUsagePerUser quotaPerUserUsageEntity) {
The method is structured of repetitive patterns, try to extract and generalize
a bit.
Line 275: if (quotaPerUserUsageEntity.isUnlimitedVcpu() ||
aggregatedUsage.isUnlimitedVcpu()) {
Line 276: aggregatedUsage.setVcpuLimit(QuotaProgressBar.UNLIMITED);
Line 277: } else {
Line 278:
aggregatedUsage.setVcpuLimit(quotaPerUserUsageEntity.getVcpuLimit() +
aggregatedUsage.getVcpuLimit());
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/main/view/tab/extended/SideTabExtendedResourceView.ui.xml
Line 40: border-color: #7a7a7a;
Line 41: padding: 1px;
Line 42: border-radius: 9px;
Line 43: background-color: white;
Line 44: box-shadow: 2px 2px 5px #888888;
Have you checked it on all browsers (e.g. chrome/ie8/ie9)?
Line 45: position: absolute;
Line 46: margin-bottom: 15px;
Line 47: }
Line 48:
Line 49: .headingContainer {
Line 50: float: left;
Line 51: background-color: #273240;
Line 52: width: 100%;
Line 53: border-radius: 9px 9px 0 0;
Have you checked it on all browsers (e.g. chrome/ie8/ie9)?
Line 54: }
Line 55:
Line 56: .iconImageContainer {
Line 57: float: left;
Line 111: width: 99%;
Line 112: margin-bottom: 15px;
Line 113: border-radius: 9px;
Line 114: background-color: white;
Line 115: box-shadow: 2px 2px 5px #888888;
same
Line 116: padding: 1px;
Line 117: }
Line 118:
Line 119: .snapshotsDataPanel {
Line 263: <g:DockLayoutPanel ui:field="mainPanel"
addStyleNames="{style.mainPanel}">
Line 264: <g:west size='280'>
Line 265: <g:DockLayoutPanel unit="PCT"
ui:field="infoContainer" addStyleNames="{style.westPanel}">
Line 266: <g:north size='50'>
Line 267: <g:FlowPanel
ui:field="infoBoxCpu" addStyleNames="{style.infoBoxCpu}">
Would be nice to extract each of these boxes to a separate widget and just
assemble them here... (e.g. infoBoxCpu+infoBoxMemory+infoBoxStorage)
Line 268:
Line 269: <g:FlowPanel
addStyleNames="{style.headingContainer}">
Line 270:
<g:FlowPanel addStyleNames="{style.iconImageContainer}">
Line 271:
<g:Image resource='{resources.cpuIcon}'/>
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/widget/DoublePercentageProgressBar.java
Line 75: public void setBars() {
Line 76: int fakeA = valueA;
Line 77: int fakeB = valueB;
Line 78:
Line 79: if (valueA != null && valueB != null && valueA + valueB >= 99)
{
consider using some constants
e.g.:
FULL_WIDTH=99
EPSILON=1
FULL_WIDTH - EPSILON (instead of 98)
Line 80: double factor = (double)98 / (valueA + valueB);
Line 81: fakeA = (int)Math.round(factor * valueA);
Line 82: fakeB = (int)Math.round(factor * valueB);
Line 83:
Line 84: fakeA = (fakeB == 0 ? 99 : fakeA);
Line 85: fakeB = (fakeA == 0 ? 99 : fakeB);
Line 86: }
Line 87:
Line 88: if (valueB != null) {
try extracting each if section to a general method
Line 89: String percentageB = valueB + "%"; //$NON-NLS-1$
Line 90: String fakePercentageB = fakeB + "%"; //$NON-NLS-1$
Line 91: percentageLabelB.setText(valueB < 10 ? "" : percentageB);
//$NON-NLS-1$
Line 92: percentageLabelB.setStyleName(style.percentageLabel());
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/widget/QuotaProgressBar.java
Line 7:
Line 8: public class QuotaProgressBar extends DoublePercentageProgressBar {
Line 9:
Line 10: public static final int UNLIMITED = -1;
Line 11: private static final CommonApplicationMessages messages =
GWT.create(CommonApplicationMessages.class);
Use 'userportal/../ApplicationMessages' instead
Line 12: private static final ApplicationConstants constants =
GWT.create(ApplicationConstants.class);
Line 13: private static final DiskSizeRenderer<Number> diskSizeRenderer =
Line 14: new
DiskSizeRenderer<Number>(DiskSizeRenderer.DiskSizeUnit.GIGABYTE);
Line 15:
Line 41: int othersConsumptionPercent = (int)
Math.round(consumedByOthers * 100 / limit);
Line 42: int userConsumptionPercent = (int) Math.round(consumedByUser *
100 / limit);
Line 43: double free = limit - consumedByOthers - consumedByUser;
Line 44:
Line 45: switch (getType()) {
might look a bit better if you extract it to a method (e.g. 'updateTitle'..)
Line 46: case STORAGE:
Line 47: String freeStorage = free == 0 ? "0" :
diskSizeRenderer.render(free); //$NON-NLS-1$
Line 48: setTitle(constants.freeStorage() + freeStorage);
Line 49: break;
Line 50: case CPU:
Line 51: setTitle(messages.quotaFreeCpus((int) free));
Line 52: break;
Line 53: case MEM:
Line 54: String freeMem = free > 4096 ?
diskSizeRenderer.render(free/1024) : (int) free + "MB"; //$NON-NLS-1$
use constants
Line 55: setTitle(constants.freeMemory() + freeMem);
Line 56: break;
Line 57: }
Line 58:
Line 68: case CPU:
Line 69:
setExceeded(messages.exceedingCpus(othersConsumptionPercent +
userConsumptionPercent - 100, (int) -free));
Line 70: break;
Line 71: case MEM:
Line 72: String freeMem = free < -4096 ?
diskSizeRenderer.render(-free/1024) : (int) -free + "MB"; //$NON-NLS-1$
use constants
Line 73:
setExceeded(messages.exceedingMem(othersConsumptionPercent +
userConsumptionPercent - 100, freeMem));
Line 74: break;
Line 75: }
Line 76: } else {
--
To view, visit http://gerrit.ovirt.org/11259
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I26ea6eb7f7ec93ffa69a2a4dea52c7e5d50dead9
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: ofri masad <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: ofri masad <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
