Alexander Wels has posted comments on this change.
Change subject: webadmin: add feedback image to webadmin
......................................................................
Patch Set 1: (4 inline comments)
You got the gist of it right, just some slight little tweaks. We use the MVP
model (Model, View, Presenter) in particular the dumb view version of that.
That basically means that there should be no logic in the view, and the
presenter contains all the logic. This includes event handlers.
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/HeaderPresenterWidget.java
Line 24:
Line 25: HasClickHandlers getConfigureLink();
Line 26:
Line 27: HasClickHandlers getAboutLink();
Line 28:
Add this here:
HasClickHandlers getFeedBackImage();
Line 29: void setFeedbackLink(String feedbackLink);
Line 30: }
Line 31:
Line 32: @ContentSlot
Line 46: WebAdminConfigurator.DOCUMENTATION_GUIDE_PATH,
dynamicMessages.applicationDocTitle());
Line 47: this.searchPanel = searchPanel;
Line 48: this.aboutPopup = aboutPopup;
Line 49: this.configurePopup = configurePopup;
Line 50: getView().setFeedbackLink(dynamicMessages.feedbackUrl());
Add this here:
registerHandler(getView().getFeedBackImage().addClickHandler(new ClickHandler()
{
@Override
public void onClick(ClickEvent event) {
if (feedbackLink != null && feedbackLink.length() > 0) {
Window.open(feedbackLink, "_blank", null); //$NON-NLS-1$
}
}
});
This automatically deregisters the event handler if the widget is destroyed,
preventing potential memory leaks.
Line 51: }
Line 52:
Line 53: @Override
Line 54: public void addTabWidget(Widget tabWidget, int index) {
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/HeaderView.java
Line 93: localize(dynamicMessages);
Line 94:
Line 95: feedbackImage.setVisible(false);
Line 96:
Line 97: feedbackImage.addClickHandler(new ClickHandler() {
Don't add the click handler in the view, add it in the presenter as I
described. This helps us keep the views 'dumb' and have all the logic in the
presenter.
Line 98:
Line 99: @Override
Line 100: public void onClick(ClickEvent event) {
Line 101: if (feedbackLink != null && feedbackLink.length() >
0) {
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/HeaderView.ui.xml
Line 29: }
Line 30:
Line 31: .menuBar {
Line 32: float: right;
Line 33: padding: 10px 140px 10px 10px;
Might want to consider making this part brand-able as well.
Line 34: }
Line 35:
Line 36: .userName {
Line 37: font-weight: bold;
--
To view, visit http://gerrit.ovirt.org/15984
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804e1ec2a7365898db7013a09639a2096a76184
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
