Vojtech Szocs has posted comments on this change.
Change subject: webadmin: Host interface manual refresh
......................................................................
Patch Set 2: Code-Review+1
(5 comments)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/AbstractRefreshManager.java
Line 139: public void onClick(ClickEvent event) {
Line 140: if (manualRefreshCallback != null) {
Line 141: manualRefreshCallback.onManualRefresh();
Line 142: }
Line 143: ManualRefreshEvent.fire(manager);
Just a nit-picky comment, this could be also:
ManualRefreshEvent.fire(AbstractRefreshManager.this);
(each anonymous class is non-static and has reference to outer/declaring class)
Line 144: controller.refresh();
Line 145: }
Line 146: });
Line 147: }
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/ManualRefreshEvent.java
Line 5: import com.google.gwt.event.shared.EventHandler;
Line 6: import com.google.gwt.event.shared.GwtEvent;
Line 7: import com.google.gwt.event.shared.HasHandlers;
Line 8:
Line 9: public class ManualRefreshEvent extends GwtEvent<ManualRefreshHandler> {
Instead of creating GWT event class by hand, you can also use @GenEvent
annotation and let GWTP's GenEventProcessor (Java annotation processor) do the
necessary boilerplate for you, for example:
package org.ovirt.engine.ui.common.widget.refresh;
@GenEvent
public class ManualRefresh {
}
This will generate ManualRefreshEvent.java in the same package as ManualRefresh
class with everything necessary (including the handler interface).
Line 10: /**
Line 11: * Manual refresh handler type.
Line 12: */
Line 13: private static final Type<ManualRefreshHandler> TYPE = new
Type<ManualRefreshHandler>();
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/host/SubTabHostInterfacePresenter.java
Line 62: }
Line 63:
Line 64: @ProxyEvent
Line 65: @Override
Line 66: public void onManualRefresh(ManualRefreshEvent event) {
The difference between doing this (1):
@Override
protected void onBind() {
super.onBind();
registerHandler(getEventBus().addHandler(ManualRefreshEvent.getType(), new
ManualRefreshHandler() {
...
}));
}
and doing this (2):
@ProxyEvent
public void onManualRefresh(ManualRefreshEvent event) {
...
}
is that (2) will cause Presenter's *Proxy* to listen for ManualRefreshEvent -
the handler will instantiate & bind Presenter and then call @ProxyEvent method
on Presenter.
In other words, (2) will enforce Presenter/View creation, i.e. in case the user
didn't reveal that Presenter/View yet (all main section presenters sit behind
split-points and are lazy-loaded as necessary).
For example, onHostSelectionChange method needs @ProxyEvent because whenever
main tab selection changes, we need to enforce Presenter/View creation and
initialize it based on main tab selection.
This is not a big deal though, since stuff like "@ProxyEvent onUiCommonInit"
will cause our sub tab Presenter/View to be created quite early on, anyway.
Line 67: getView().removeContent();
Line 68: }
Line 63:
Line 64: @ProxyEvent
Line 65: @Override
Line 66: public void onManualRefresh(ManualRefreshEvent event) {
Line 67: getView().removeContent();
Maybe we can update View only if we are visible on screen:
if (isVisible()) {
getView().removeContent();
}
Otherwise, if you trigger manual refresh on any main tab,
SubTabHostInterfaceView will have its content removed even though it's not
visible (i.e. even though you are in main tab other than "Hosts").
Line 68: }
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/host/SubTabHostInterfaceView.java
Line 28: import com.google.gwt.user.client.ui.VerticalPanel;
Line 29:
Line 30: public class SubTabHostInterfaceView extends
AbstractSubTabFormView<VDS, HostListModel, HostInterfaceListModel>
Line 31: implements SubTabHostInterfacePresenter.ViewDef {
Line 32: HostInterfaceForm hostInterfaceForm = null;
Please move this field a little bit below, i.e. next to "contentPanel" field,
to preserve common member ordering [nested types, fields, constructors,
methods].
Line 33:
Line 34: interface ViewIdHandler extends
ElementIdHandler<SubTabHostInterfaceView> {
Line 35: ViewIdHandler idHandler = GWT.create(ViewIdHandler.class);
Line 36: }
--
To view, visit http://gerrit.ovirt.org/19143
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3e38c077a6eeef335fe1f8e8f468c70a5ca8ad92
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
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
