Alexander Wels has posted comments on this change. Change subject: userportal,webadmin: Persist advanced/basic mode per dialog type ......................................................................
Patch Set 1: (1 comment) A couple of notes: # It seems that sometimes the storage key comes from a method and other times it is just some string from somewhere, is there any rhyme or reasoning to it. Can we maybe in a base class always call some method, and each sub class just override that method to provide the key value? # You are passing LocalStorage to all kinds of presenters, due to LocalStorage being browser specific you now have essentially made it impossible to unit test the presenters. Would it be possible to make a LocalStorage interface and pass that into the presenters, then we can mock the interface and still test the presenters. This is something we probably should have done a while ago. http://gerrit.ovirt.org/#/c/26170/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/TemplateListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/TemplateListModel.java: Line 444: } Line 445: Line 446: private void vmInitLoaded(VmTemplate template) { Line 447: UnitVmModel model = new UnitVmModel(createBehavior(template)); Line 448: model.setIsAdvancedModeLocalStorageKey(getEditTemplateAdvancedModelKey()); //$NON-NLS-1$ Don't need the //$NON-NLS-1$ here since you are calling the method. Line 449: setWindow(model); Line 450: model.setTitle(ConstantsManager.getInstance().getConstants().editTemplateTitle()); Line 451: model.setHelpTag(HelpTag.edit_template); Line 452: model.setHashName("edit_template"); //$NON-NLS-1$ -- To view, visit http://gerrit.ovirt.org/26170 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I786daf31eb5cf713bf5b7afe1210d2655ef7bd4a Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
