Vojtech Szocs has posted comments on this change.
Change subject: userportal: Dynamic guide link
......................................................................
Patch Set 1:
(5 comments)
> Since, in ovirt, the links are the same, we should change the message to say
> something like "added the *ability to have* different links."
+1
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/tab/AbstractTabPanel.java
Line 22: * <ul>
Line 23: * <li>{@link #tabContentContainer} widget for displaying tab contents
Line 24: * </ul>
Line 25: */
Line 26: public abstract class AbstractTabPanel extends Composite implements
TabPanel, DynamicTabPanel, HasHandlers {
Why is this change necessary?
Line 27:
Line 28: @UiField
Line 29: public Panel tabContentContainer;
Line 30:
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/ApplicationDynamicMessages.java
Line 26: *
Line 27: * @return The guide URL.
Line 28: */
Line 29: public final String extendedGuideUrl() {
Line 30: return formatString(DynamicMessageKey.EXTENDED_GUIDE_URL,
LocaleInfo.getCurrentLocale().getLocaleName());
We already do "LocaleInfo.getCurrentLocale().getLocaleName()" in
DynamicMessages, please consider adding a method like this:
protected LocaleInfo getCurrentLocale() {
return LocaleInfo.getCurrentLocale();
}
into DynamicMessages and use this method in both places, i.e.
DynamicMessages.guideUrl + ApplicationDynamicMessages.extendedGuideUrl
Line 31: }
Line 32:
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/gin/SystemModule.java
Line 71: }
Line 72:
Line 73: @Provides
Line 74: @Inject
Line 75: public UserPortalPlaceManager getUserPortalPlaceManager(EventBus
eventBus, TokenFormatter tokenFormatter,
Can you please try annotating this method with @Singleton and debug to find out
if it gets called only once? If so, we could just implement this method like
this:
return new UserPortalPlaceManager(...);
and avoid having "singletonPlaceManager" static field altogether (GIN-generated
code should do the singleton null instance check for us).
But I may be wrong here.
Line 76: CurrentUser user, CurrentUserRole userRole,
Line 77: @DefaultLoginSectionPlace String defaultLoginSectionPlace,
Line 78: @DefaultMainSectionPlace String defaultMainSectionPlace,
Line 79: @DefaultMainSectionExtendedPlace String
defaultMainSectionExtendedPlace) {
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/main/presenter/HeaderPresenterWidget.java
Line 14: import com.google.inject.Inject;
Line 15: import com.gwtplatform.mvp.client.proxy.RevealRootPopupContentEvent;
Line 16:
Line 17: public class HeaderPresenterWidget extends
AbstractHeaderPresenterWidget<HeaderPresenterWidget.ViewDef>
Line 18: implements TabWidgetHandler {
Why is this change necessary?
Line 19:
Line 20: public interface ViewDef extends
AbstractHeaderPresenterWidget.ViewDef, TabWidgetHandler {
Line 21:
Line 22: HasClickHandlers getAboutLink();
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/gin/SystemModule.java
Line 54: }
Line 55:
Line 56: @Provides
Line 57: @Inject
Line 58: public PlaceManager getPlaceManager(EventBus eventBus,
TokenFormatter tokenFormatter, CurrentUser user,
Some questions:
* Did you verify that double-PlaceManager issue eventually occurs also in
WebAdmin, i.e. placing breakpoint in WebAdminPlaceManager constructor that gets
hit multiple times? [in other words - do we need this @Provides method at all?]
* Isn't the single @Provides "PlaceManager getPlaceManager" method equivalent
to following binding?
bind(PlaceManager.class).to(WebAdminPlaceManager.class);
* As suggested in UserPortal's SystemModule.java - can you please try to
annotate this method with @Singleton and debug to find out if it gets called
only once? If so, we could just implement this method like this:
return new WebAdminPlaceManager(...);
and avoid having "singletonPlaceManager" static field altogether (GIN-generated
code should do the singleton null instance check for us).
Line 59: @DefaultLoginSectionPlace String defaultLoginSectionPlace,
Line 60: @DefaultMainSectionPlace String defaultMainSectionPlace) {
Line 61: if (singletonPlaceManager == null) {
Line 62: singletonPlaceManager = new WebAdminPlaceManager(eventBus,
tokenFormatter, user, defaultLoginSectionPlace,
--
To view, visit http://gerrit.ovirt.org/19298
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If6374df0da70fa6f6edf044d4ebadafc7fb1a105
Gerrit-PatchSet: 1
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
