Vojtech Szocs has posted comments on this change.
Change subject: webadmin: Improve UI Plugin tab API
......................................................................
Patch Set 2: (2 inline comments)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/presenter/AbstractSubTabPresenter.java
Line 115: // Notify model provider that the tab has been revealed
Line 116: modelProvider.onSubTabSelected();
Line 117:
Line 118: if (getTable() != null) {
Line 119: getTable().setLoadingState(LoadingState.LOADING);
getTable() isn't really expensive, both main tab & sub tab presenters implement
it as "getView().getTable()" and corresponding view implements getTable() by
returning "final" table widget reference.
Besides, before this patch, the situation was the same, but with
getView().getTable() :-)
Line 120: }
Line 121: }
Line 122:
Line 123: @Override
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/cluster/SubTabClusterPolicyPresenter.java
Line 44: ViewDef view,
Line 45: ProxyDef proxy,
Line 46: PlaceManager placeManager,
Line 47: DetailModelProvider<ClusterListModel, ClusterPolicyModel>
modelProvider,
Line 48: ApplicationConstants constants) {
> Do we need the application constants injected if we don't use it anywhere?
Good point - indeed, we don't need ApplicationConstants constructor argument. I
removed "constants" field because it was unused, and forgot to remove
corresponding constructor argument.
> For that matter if we don't need it injected, do we even need this
> constructor? all it would do is just call super.
Yes, we still need our @Inject constructor, even if it just calls super.
AbstractSubTabPresenter accepts dependencies via constructor and we need to
@Inject and call super to pass these dependencies to parent class.
Line 49: super(eventBus, view, proxy, placeManager, modelProvider);
Line 50: }
Line 51:
Line 52: @Override
--
To view, visit http://gerrit.ovirt.org/15810
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f3e33844693a08352cc54b60b05b3f6ac5eb816
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Keith Robertson <[email protected]>
Gerrit-Reviewer: Spenser Shumaker <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
