Better coverage of Changed event

Author: k1o0

+eui/ConditionPanel.m

@@ -169,6 +169,7 @@ function deleteSelectedConditions(obj)
           obj.makeGlobal;
       else % Otherwise delete the selected conditions as usual
           obj.ParamEditor.Parameters.removeConditions(rows);
+          notify(obj.ParamEditor, 'Changed')
       end
       % Refresh the table of conditions
       obj.fillConditionTable();

+eui/FieldPanel.m

@@ -45,7 +45,9 @@
       props.HorizontalAlignment = 'left';
       props.UIContextMenu = obj.ContextMenu;
       props.Parent = obj.UIPanel;
-      label = uicontrol('Style', 'text', 'String', name, props);
+      props.Tag = name;
+      title = obj.ParamEditor.Parameters.title(name);
+      label = uicontrol('Style', 'text', 'String', title, props);
       if nargin < 3
         ctrl = uicontrol('Style', 'edit', props);
       end
@@ -56,6 +58,14 @@
     end
 
     function onEdit(obj, src, id)
+      % ONEDIT Callback for edits to field controls
+      %  Updates the underlying parameter struct, changes the UI
+      %  value/string and changes the label colour to red. The src object
+      %  should be the edit or checkbox ui control that has been edited,
+      %  and id is the unformatted parameter name (stored in the Tag
+      %  property of the label and control elements).
+      %
+      % See also ADDFIELD, EUI.PARAMEDITOR/UPDATE
       disp(id);
       switch get(src, 'style')
         case 'checkbox'
@@ -67,11 +77,14 @@ function onEdit(obj, src, id)
           newValue = obj.ParamEditor.update(id, get(src, 'string'));
           set(src, 'String', obj.ParamEditor.paramValue2Control(newValue));
       end
-      changed = strcmp(id,{obj.Labels.String});
+      changed = strcmp(src.Tag,{obj.Labels.Tag});
       obj.Labels(changed).ForegroundColor = [1 0 0];
     end
 
     function clear(obj, idx)
+      % CLEAR Delete a parameter field
+      %  Deletes the label and control elements at index idx.  If no index
+      %  given, all controls are deleted.  
       if nargin == 1
         idx = true(size(obj.Labels));
       end
@@ -99,18 +112,24 @@ function makeConditional(obj, name)
         selected = obj.ParamEditor.getSelected();
         if isa(selected, 'matlab.ui.control.UIControl') && ...
             strcmp(selected.Style, 'text')
-          name = selected.String;
+          name = selected.Tag;
         else % Assume control
-          name = obj.Labels([obj.Controls]==selected).String;
+          name = obj.Labels([obj.Controls]==selected).Tag;
         end
       end
-      idx = strcmp(name,{obj.Labels.String});
+      idx = strcmp(name,{obj.Labels.Tag});
       assert(~ismember(name, {'randomiseConditions'}), ...
         '%s can not be made a conditional parameter', name)
 
       obj.clear(idx);
+      % FIXME The below code could be in a makeConditional method of
+      % eui.ParamEditor, thus more clearly separating class functionality:
+      % Editing the exp.Parameters object directly should only be done by
+      % ParamEditor.  This would also make subclassing these panel classes
+      % more straightforward
       obj.ParamEditor.Parameters.makeTrialSpecific(name);
       obj.ParamEditor.ConditionalUI.fillConditionTable();
+      notify(obj.ParamEditor, 'Changed');
       obj.onResize;
     end
 

+eui/MControl.m

@@ -321,6 +321,9 @@ function loadParamProfile(obj, profile)
     end
 
     function paramChanged(obj)
+      % PARAMCHANGED Indicate to user that parameters have been updated
+      %  Changes the label above the ParamEditor indicating that the
+      %  parameters have been edited
       s = get(obj.ParamProfileLabel, 'String');
       if ~strEndsWith(s, '[EDITED]')
         set(obj.ParamProfileLabel, 'String', [s ' ' '[EDITED]'], 'ForegroundColor', [1 0 0]);

+eui/ParamEditor.m

@@ -1,30 +1,54 @@
 classdef ParamEditor < handle
-  %UNTITLED2 Summary of this class goes here
-  %   ParamEditor deals with setting the paramters via a GUI
+  %PARAMEDITOR GUI for visualizing and editing experiment parameters
+  %   ParamEditor deals with setting the paramters via a GUI.  In general
+  %   this class is involved in constructing a UI comprising a Global UI
+  %   panel, handled by the EUI.FIELDPANEL class, and a Condition table,
+  %   handled by the EUI.CONDITIONPANEL class.  It is also responsible for
+  %   updating the underlying EXP.PARAMETERS object and notifying
+  %   downstream listeners of parameter changes.
+  %
+  % See also EUI.FIELDPANEL, EUI.CONDITIONPANEL
 
   properties
+    % An exp.Parameters object, which keeps track of all parameter changes
     Parameters
   end
 
   properties (Access = {?eui.ConditionPanel, ?eui.FieldPanel})
-    UIPanel
+    % Handle to the EUI.FIELDPANEL object, which manages the display of the
+    % Global parameters
     GlobalUI
+    % Handle to the EUI.CONDITIONPANEL object, which manages the display of
+    % the trial conditions within a ui table
     ConditionalUI
+    % Handle to the parent container for the ParamEditor.  If constructor
+    % called with no parant input, then this will be a figure handle, the
+    % same as Root
     Parent
+    % Handle to the figure within which the ParamEditor is displayed
     Root
+    % A listener for changes to the figure size.  See also ONRESIZE
     Listener
   end
 
   properties (Dependent)
+    % Flag for making editor read only by disabling all UI controls
     Enable
   end
 
   events
+    % Event notified each time a user makes an edit to a parameter
     Changed
   end
 
   methods
     function obj = ParamEditor(pars, parent)
+      % PARAMEDITOR GUI for visualizing and editing experiment parameters
+      %  The input pars is expected to be an instance of the exp.Parameters
+      %  class.  Parant is a handle to a parent figure or UI Panel.  If no
+      %  parant is given, the editor is created in a new figure.
+      %
+      % See also EUI.FIELDPANEL, EUI.CONDITIONPANEL
       if nargin == 0; pars = []; end
       if nargin < 2
         parent = figure('Name', 'Parameters', 'NumberTitle', 'off',...
@@ -55,6 +79,10 @@
     end
 
     function delete(obj)
+      % DELETE Deletes all panels
+      %  Called when the ParamEditor object is deleted or its parant figure
+      %  is closed.  Deletes all UI elements and data.
+      % See also CLEAR
       delete(obj.GlobalUI);
       delete(obj.ConditionalUI);
     end
@@ -79,11 +107,27 @@ function delete(obj)
     end
 
     function clear(obj)
+      % CLEAR Clear the Global and Condition panels
+      %  Deletes all UI fields (labels and control elements) and clears the
+      %  Condition Table data.
+      % 
+      % See also BUILDUI
       clear(obj.GlobalUI);
       clear(obj.ConditionalUI);
     end
 
     function buildUI(obj, pars)
+      % BUILDUI Populate Global and Condition UI panels with paramter set
+      %  Clears any existing fields and the condition table, then loads new
+      %  UI controls and labels for the Global parameters, and fills the
+      %  condition table.  The input pars must be an instance of the
+      %  exp.Parameters class.
+      %
+      %  RandomiseConditions is not added as a field but instead is
+      %  represented as a context menu item
+      %
+      % See also EXP.PARAMETERS, EUI.FIELDPANEL/ADDFIELD,
+      % EUI.CONDITIONPANEL/FILLCONDITIONTABLE
       obj.Parameters = pars;
       obj.clear() % Clear the current parameter UI elements
       if isempty(pars); return; end % Nothing to build
@@ -95,7 +139,7 @@ function buildUI(obj, pars)
         if strcmp(nm, 'randomiseConditions'); continue; end
         if islogical(pars.Struct.(nm{:})) % If parameter is logical, make checkbox
           ctrl = uicontrol('Parent', c.UIPanel, 'Style', 'checkbox', ...
-            'Value', pars.Struct.(nm{:}));
+            'Value', pars.Struct.(nm{:}), 'Tag', nm{:});
           addField(c, nm{:}, ctrl);
         else % Otherwise create the default field; a text box
           [~, ctrl] = addField(c, nm{:});
@@ -166,6 +210,12 @@ function addEmptyConditionToParam(obj, name)
     end
 
     function newValue = update(obj, name, value, row)
+      % UPDATE Updates the exp.Parameters object with new value
+      %  Called when either the Condition table or Global param fields are
+      %  updated by the user.  Updated the underlying paramters structure
+      %  and notifies listeners of the change via the Changed event.
+      %
+      % See also EUI.FIELDPANEL/ONEDIT, EUI.CONDITIONPANEL/ONEDIT
       if nargin < 4; row = 1; end
       currValue = obj.Parameters.Struct.(name)(:,row);
       if iscell(currValue)

tests/ParamEditorTest.m

@@ -9,10 +9,14 @@
     Figure
     % Handle to trial conditions UI Table
     Table
+    % Test parameter structure
+    Parameters
   end
 
-  properties %(MethodSetupParameter)
-    Parameters
+  properties (SetAccess = private)
+    % Flag set to true each time the ParamEditor's Changed event is
+    % notified
+    Changed = false
   end
 
   methods (TestClassSetup)
@@ -21,7 +25,7 @@ function setup(testCase)
       testCase.FigureVisibleDefault = get(0,'DefaultFigureVisible');
 %       set(0,'DefaultFigureVisible','off');
       testCase.addTeardown(@set, 0, 'DefaultFigureVisible', testCase.FigureVisibleDefault);
-      
+            
       % Loads validation data
       %  Graph data is a cell array where each element is the graph number
       %  (1:3) and within each element is a cell of X- and Y- axis values
@@ -59,6 +63,13 @@ function buildParams(testCase)
       % Ensure all conditional params have column in table
       testCase.fatalAssertTrue(isequal(size(testCase.Table.Data), ...
         [PE.Parameters.numTrialConditions, numel(PE.Parameters.TrialSpecificNames)]))
+      % Add callback for verifying that Changed event listeners are
+      % notified
+      callback = @(~,~)testCase.setChanged(true);
+      lh = event.listener(testCase.ParamEditor, 'Changed', callback);
+      testCase.addTeardown(@delete, lh);
+      % Reset the Changed flag
+      testCase.Changed = false;
     end
   end
 
@@ -69,6 +80,7 @@ function test_makeConditional(testCase)
       % conditional, and that the underlying Parameters object is also
       % affected
       PE = testCase.ParamEditor;
+      testCase.assertTrue(~testCase.Changed, 'Changed flag incorrect')
       % Number of global parameters: find all text labels
       gLabels = @()findobj(testCase.Figure, 'Style', 'text');
       gInputs = @()findobj(testCase.Figure, 'Style', 'checkbox', '-or', 'Style', 'edit');
@@ -80,9 +92,12 @@ function test_makeConditional(testCase)
       c = findobj(testCase.Figure, 'Text', 'Make Conditional');
       % Set the focused object to one of the parameter labels
       set(testCase.Figure, 'CurrentObject', ...
-        findobj(testCase.Figure, 'String', 'rewardVolume'))
+        findobj(testCase.Figure, 'Tag', 'rewardVolume'))
       testCase.verifyWarningFree(c.MenuSelectedFcn, ...
         'Problem making parameter conditional');
+      % Verify Changed event triggered
+      testCase.verifyTrue(testCase.Changed, ...
+        'Failed to notify listeners of parameter change')
       % Verify change in UI elements
       testCase.verifyTrue(numel(gLabels()) == nGlobalLabels-1, ...
         'Global parameter UI element not removed')
@@ -152,15 +167,17 @@ function test_paramValue2Control(testCase)
 
     function test_newCondition(testCase)
       PE = testCase.ParamEditor;
-      
       tableRows = size(testCase.Table.Data, 1);
 
       % Make function handle param conditional to test default value
       % Set the focused object to one of the parameter labels
       set(testCase.Figure, 'CurrentObject', ...
-        findobj(testCase.Figure, 'String', 'experimentFun'))
+        findobj(testCase.Figure, 'Tag', 'experimentFun'))
       feval(pick(findobj(testCase.Figure, 'Text', 'Make Conditional'), 'MenuSelectedFcn'))
 
+      % Reset Changed flag
+      testCase.Changed = false;
+      
       % Retrieve function handle for new condition
       fn = pick(findobj(testCase.Figure, 'String', 'New condition'), 'Callback');
       testCase.verifyWarningFree(fn, 'Warning encountered adding trial condition')
@@ -177,11 +194,15 @@ function test_newCondition(testCase)
       [~, trialParams] = PE.Parameters.assortForExperiment;
       testCase.verifyTrue(isequal(struct2cell(trialParams(end)), ...
         {@nop; zeros(2,1); zeros(3,1); false; 0}))
+      
+      % Verify listeners WEREN'T notified
+      testCase.verifyTrue(~testCase.Changed, ...
+        'Shouldn''t have notified listeners of parameter change')
     end
 
     function test_deleteCondition(testCase)
       PE = testCase.ParamEditor;
-      
+      testCase.assertTrue(~testCase.Changed, 'Changed flag incorrect')
       tableRows = size(testCase.Table.Data, 1);
       % Select some cells to delete
       event.Indices = [(1:5)' ones(5,1)];
@@ -200,6 +221,10 @@ function test_deleteCondition(testCase)
       testCase.assertEqual(size(testCase.Table.Data), ...
         [PE.Parameters.numTrialConditions, numel(PE.Parameters.TrialSpecificNames)])
 
+      % Verify Changed event triggered
+      testCase.verifyTrue(testCase.Changed, ...
+        'Failed to notify listeners of parameter change')
+      
       % Test behaviour when < 2 conditions remain
       event.Indices = [(1:PE.Parameters.numTrialConditions-1)' ...
         ones(PE.Parameters.numTrialConditions-1,1)];
@@ -216,13 +241,14 @@ function test_deleteCondition(testCase)
 
     function test_setValues(testCase)
       % TODO Add test for the set values button.  For now let's fail this
+      testCase.assertTrue(~testCase.Changed, 'Changed flag incorrect')
       PE = testCase.ParamEditor;
       testCase.assertTrue(false, 'Test not implemented')
     end
 
     function test_globaliseParam(testCase)
       PE = testCase.ParamEditor;
-      
+      testCase.assertTrue(~testCase.Changed, 'Changed flag incorrect')
       tableCols = size(testCase.Table.Data, 2);
       % Globalize one param
       event.Indices = [1, 2];
@@ -240,6 +266,10 @@ function test_globaliseParam(testCase)
       testCase.verifyEqual(size(testCase.Table.Data), ...
         [PE.Parameters.numTrialConditions, numel(PE.Parameters.TrialSpecificNames)])
 
+      % Verify Changed event triggered
+      testCase.verifyTrue(testCase.Changed, ...
+        'Failed to notify listeners of parameter change')
+      
       % Test removal of all but numRepeats: numRepeats should automatically
       % globalize
       n = numel(PE.Parameters.TrialSpecificNames)-1;
@@ -280,6 +310,7 @@ function test_globaliseParam(testCase)
     function test_paramEdits(testCase)
       % Test basic edits to Global UI controls and Condition table
       PE = testCase.ParamEditor;
+      testCase.assertTrue(~testCase.Changed, 'Changed flag incorrect')
 
       % Retreive all global parameters labels and input controls
       gLabels = findobj(testCase.Figure, 'Style', 'text');
@@ -302,6 +333,10 @@ function test_paramEdits(testCase)
       par = strcmpi(PE.Parameters.GlobalNames, strrep(gLabels(idx).String, ' ', ''));
       testCase.verifyEqual(PE.Parameters.Struct.(PE.Parameters.GlobalNames{par}), 666, ...
         'UI edit failed to update parameters struct')
+      % Verify Changed event triggered
+      testCase.verifyTrue(testCase.Changed, ...
+        'Failed to notify listeners of parameter change')
+      testCase.Changed = false;
 
       % Test editing global param, 'checkbox' UI
       idx = find(strcmp({gInputs.Style}, 'checkbox'), 1);
@@ -319,6 +354,10 @@ function test_paramEdits(testCase)
       testCase.verifyEqual(...
         PE.Parameters.Struct.(PE.Parameters.GlobalNames{par}), gInputs(idx).Value==true, ...
         'UI checkbox failed to update parameters struct')
+      % Verify Changed event triggered
+      testCase.verifyTrue(testCase.Changed, ...
+        'Failed to notify listeners of parameter change')
+      testCase.Changed = false;
 
       % Test edits to conditions table
       callback_fcn = testCase.Table.CellEditCallback;
@@ -333,6 +372,9 @@ function test_paramEdits(testCase)
       value = PE.Parameters.Struct.(PE.Parameters.TrialSpecificNames{1});
       testCase.verifyEqual(value(:,1), [0;5], ...
         'Table UI failed to update parameters struct')
+      % Verify Changed event triggered
+      testCase.verifyTrue(testCase.Changed, ...
+        'Failed to notify listeners of parameter change')
     end
 
     function test_interactivity(testCase)
@@ -374,4 +416,10 @@ function test_interactivity(testCase)
 
   end
 
+  methods
+    function setChanged(testCase, value)
+      testCase.Changed = value;
+    end
+  end
+  
 end