Skip to content

Refactor AppPlugin architecture and remove legacy imviewer plugins path#73

Open
ehennestad wants to merge 16 commits intodevfrom
dev-refactor-app-plugin
Open

Refactor AppPlugin architecture and remove legacy imviewer plugins path#73
ehennestad wants to merge 16 commits intodevfrom
dev-refactor-app-plugin

Conversation

@ehennestad
Copy link
Copy Markdown
Collaborator

@ehennestad ehennestad commented Mar 28, 2026

Summary

  • remove the legacy lowercase plugins path from imviewer and make PointerManager app-owned
  • centralize plugin key and key-release dispatch through AppWithPlugin
  • split plugin options from UserSettings and move true settings ownership to concrete classes that actually need it
  • rename the modal preview workflow layer to ModalMethodPreviewController and migrate modal and live plugins onto options-oriented APIs
  • introduce AppBridgePlugin and migrate RoiClassifier onto bridge/controller semantics
  • clean up the base plugin contract by removing stale settings-era and icon-era assumptions

Main changes

  • imviewer.App now owns PointerManager directly
  • RoiManager no longer looks up pointer tools through the legacy plugin registry
  • AppPlugin no longer inherits UserSettings
  • RoiManager now inherits UserSettings directly as a true settings owner
  • NoRMCorre, FlowRegistration, FluFinder, EXTRACT, DffExplorer, and CaimanDeconvolution now use options-oriented plugin/controller APIs
  • RoiClassifier now uses AppBridgePlugin

Notes

  • This PR is structurally complete from the refactor perspective, but runtime MATLAB verification is still recommended for plugin open and close flows and preview workflows.
  • Planning and research notes for this work live under the .codex/subprojects/2026-03-27-refactor-app-plugin directory in the branch history.

ehennestad and others added 15 commits March 27, 2026 15:28
- Add explicit PointerManager property (accessible to ImviewerPlugin subclasses)
- Add initialisePointerManager private method; call it on init and figure recreation
- Replace all obj.plugins lookups for pointerManager with direct obj.PointerManager access
- Remove the lowercase plugins property (no remaining call sites)
- Update RoiManager to fetch PointerManager from hViewer.PointerManager directly

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(Phase 2+3)

- Add sendKeyReleaseEventToPlugins to AppWithPlugin, parallel to sendKeyEventToPlugins
- Call sendKeyReleaseEventToPlugins in imviewer.App.onKeyReleased so plugins receive key release events through the new path
- Remove legacy plugins struct aggregation from UserSettings.editSettings; composite settings aggregation now belongs on the app/controller side
- Simplify editSettings: remove doDefault flag, fix indentation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Restores the behaviour removed from UserSettings.editSettings, now
correctly using obj.Plugins (AppWithPlugin) instead of the removed
lowercase plugins struct.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- New applify.mixin.HasOptionsManager: instance-level OptionsManager,
  dependent Options property, editOptions method, and onOptionsChanged
  hook for subclasses — clean separation from UserSettings
- DffExplorer now inherits HasOptionsManager instead of using the
  settings workaround: assignDefaultOptions only sets OptionsManager,
  onOptionsChanged replaces onSettingsChanged, editOptions replaces
  editSettings in the constructor

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove PrimaryAppName abstract property (assigned by subclasses but
  never read by the base or AppWithPlugin)
- Remove dead methods(Abstract,Static) block with commented-out getPluginIcon
- Remove commented-out onPluginActivated abstract methods block
- Remove stale Todo comment (resolved by HasOptionsManager)
- Add concise comment explaining the class role and why Heterogeneous is needed
- Make singleton constructor behaviour explicit: warn and return early
  instead of silently substituting a different object mid-construction

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove assignDataIoModel (body was immediate return, fully dead)
- Remove NumFrames dependent property (no get method, never used)
- Remove redundant PrimaryApp and Axes assignments from constructor;
  both are already set via onPluginActivated during activatePlugin
- Remove double onImviewerSet call (constructor no longer calls it
  directly; onPluginActivated is the single call site)
- Remove stale %PointerManager comment (now a real property on App)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gin (Phase 7)

- New applify.mixin.ModalMethodPreviewPlugin: owns RunMethodOnFinish,
  DestroyOnFinish, Modal, editSettings, openSettingsEditor,
  onSettingsEditorResumed, onSettingsEditorClosed, place, relocatePrimaryApp
- AppPlugin loses all of the above; now contains only core plugin
  lifecycle: Name, PrimaryApp, PartialConstruction, key callbacks,
  activatePlugin, DataIoModel, OptionsManager, MenuItem, Icon
- NoRMCorre, FlowRegistration, EXTRACT, FluFinder now inherit
  ModalMethodPreviewPlugin alongside ImviewerPlugin
- Remove dead loadSettings/saveSettings no-op overrides from NoRMCorre,
  EXTRACT, FluFinder (USE_DEFAULT_SETTINGS=false already prevents file I/O)
- Remove dead getPluginIcon static stubs from EXTRACT and FluFinder
- Migrate CaimanDeconvolution to HasOptionsManager; remove settings workaround

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rename methods to drop the settings terminology:
  editSettings        -> editOptions
  openSettingsEditor  -> openOptionsEditor
  onSettingsEditorResumed -> onOptionsEditorResumed
  onSettingsEditorClosed  -> onOptionsEditorClosed

Update all overrides and call sites in NoRMCorre, FlowRegistration,
EXTRACT and FluFinder. Fix superclass qualifier in NoRMCorre and
FlowRegistration to reference ModalMethodPreviewPlugin directly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
wasAborted should be public
Fix syntax error

function optionsEditor = openOptionsEditor(obj)
%openOptionsEditor Open the ui dialog for editing method options.
titleStr = sprintf('Options Editor (%s)', obj.Name);

Check warning

Code scanning / Code Analyzer

'Name' is referenced but is not a property, method, or event name defined in this class. Warning

'Name' is referenced but is not a property, method, or event name defined in this class.
function optionsEditor = openOptionsEditor(obj)
%openOptionsEditor Open the ui dialog for editing method options.
titleStr = sprintf('Options Editor (%s)', obj.Name);
if ~isempty(obj.OptionsManager)

Check warning

Code scanning / Code Analyzer

'OptionsManager' is referenced but is not a property, method, or event name defined in this class. Warning

'OptionsManager' is referenced but is not a property, method, or event name defined in this class.
%openOptionsEditor Open the ui dialog for editing method options.
titleStr = sprintf('Options Editor (%s)', obj.Name);
if ~isempty(obj.OptionsManager)
optionsEditor = obj.OptionsManager.openOptionsEditor([], obj.getCurrentOptions());

Check warning

Code scanning / Code Analyzer

'OptionsManager' is referenced but is not a property, method, or event name defined in this class. Warning

'OptionsManager' is referenced but is not a property, method, or event name defined in this class.
obj.hOptionsEditor = [];
obj.onOptionsEditorClosed()
if ~obj.wasAborted && obj.RunMethodOnFinish
obj.run()

Check warning

Code scanning / Code Analyzer

'run' is referenced but is not a property, method, or event name defined in this class. Warning

'run' is referenced but is not a property, method, or event name defined in this class.
%relocatePrimaryApp Shift app and plugin windows so they do not overlap.
if nargin < 3; direction = 'horizontal'; end
figPos(1,:) = getpixelposition(hPlugin.Figure);
figPos(2,:) = getpixelposition(obj.PrimaryApp.Figure);

Check warning

Code scanning / Code Analyzer

'PrimaryApp' is referenced but is not a property, method, or event name defined in this class. Warning

'PrimaryApp' is referenced but is not a property, method, or event name defined in this class.
if nargin < 3; direction = 'horizontal'; end
figPos(1,:) = getpixelposition(hPlugin.Figure);
figPos(2,:) = getpixelposition(obj.PrimaryApp.Figure);
hFig = [hPlugin.Figure, obj.PrimaryApp.Figure];

Check warning

Code scanning / Code Analyzer

'PrimaryApp' is referenced but is not a property, method, or event name defined in this class. Warning

'PrimaryApp' is referenced but is not a property, method, or event name defined in this class.
case 'horizontal'; figPos_ = figPos(:, [1,3]); dim = 1;
case 'vertical'; figPos_ = figPos(:, [2,4]); dim = 2;
end
screenPos = uim.utility.getCurrentScreenSize(obj.PrimaryApp.Figure);

Check warning

Code scanning / Code Analyzer

'PrimaryApp' is referenced but is not a property, method, or event name defined in this class. Warning

'PrimaryApp' is referenced but is not a property, method, or event name defined in this class.
function options = get.Options(obj)
if ~isempty(obj.Options_)
options = obj.Options_;
elseif ~isempty(obj.OptionsManager)

Check warning

Code scanning / Code Analyzer

'OptionsManager' is referenced but is not a property, method, or event name defined in this class. Warning

'OptionsManager' is referenced but is not a property, method, or event name defined in this class.
if ~isempty(obj.Options_)
options = obj.Options_;
elseif ~isempty(obj.OptionsManager)
options = obj.OptionsManager.Options;

Check warning

Code scanning / Code Analyzer

'OptionsManager' is referenced but is not a property, method, or event name defined in this class. Warning

'OptionsManager' is referenced but is not a property, method, or event name defined in this class.
@github-actions
Copy link
Copy Markdown

Test Results

112 tests  ±0   112 ✅ ±0   16s ⏱️ ±0s
  7 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 39f09f6. ± Comparison against base commit 69ef7df.

@ehennestad
Copy link
Copy Markdown
Collaborator Author

ehennestad commented Mar 28, 2026

Manual checklist

1. imviewer Core

Open imviewer with a representative stack and check:

  • app opens without startup errors
  • PointerManager exists and default pointer behavior works
  • toolbar zoomIn works
  • toolbar zoomOut works
  • toolbar pan works
  • pointer icon updates when entering/leaving the image area
  • key press handling still works for active tools
  • key release handling still works for active tools
  • recreating or refreshing the figure does not break pointer tools

2. App Settings

From imviewer, open the settings/preferences UI and check:

  • app settings dialog opens
  • app settings can be changed and applied
  • app settings save without error
  • if plugin settings are present, the composite settings flow still works

3. RoiManager

Open RoiManager from imviewer and check:

  • plugin opens without constructor errors
  • toolbar for ROI tools appears
  • pointer-based ROI tools initialize correctly
  • switching between ROI pointer tools works
  • RoiManager settings dialog opens
  • RoiManager settings changes apply correctly
  • RoiManager settings save/load still work

4. RoiClassifier

Launch RoiClassifier from both entry points if available:

  • open from the app/taskbar/button path
  • open from the RoiManager path
  • classifier app opens successfully
  • closing the classifier also cleans up the bridge plugin
  • reopening after closing works repeatedly
  • failed launch does not leave a dead active plugin behind

5. Modal Method Preview Plugins

For each of the following, open the plugin from imviewer and verify:

NoRMCorre

  • options editor opens
  • options can be changed without callback errors
  • preview/test run works
  • show-results preview works
  • save-results preview works
  • canceling the editor cleans up preview state
  • confirming the editor runs the method when expected
  • closing the plugin/editor destroys the plugin when expected

FlowRegistration

  • options editor opens
  • options can be changed without callback errors
  • preview/test run works
  • Gaussian filter is applied while editing when expected
  • Gaussian filter is reset on close/cancel
  • confirm/run path works
  • cancel/close path cleans up correctly

FluFinder

  • options editor opens
  • preview mode dropdown changes the displayed preview correctly
  • preprocessing preview updates correctly
  • binarized preview updates correctly
  • background image updates correctly when relevant options change
  • template/cell overlay updates correctly
  • cancel/close path cleans up correctly

EXTRACT

  • options editor opens
  • grid preview updates when partition settings change
  • cell template preview updates when radius changes
  • warning/help messages still appear for relevant options
  • confirm/run path works
  • cancel/close path cleans up correctly

6. Live Signalviewer Plugins

Test these from the signal viewer context:

DffExplorer

  • plugin opens
  • options editor opens
  • changing options updates the visible signal workflow
  • repeated edits do not leave stale UI/plugin state

CaimanDeconvolution

  • plugin opens
  • options editor opens
  • changing options updates deconvolution behavior
  • update-visible-only mode works
  • update-all mode works
  • plotted overlays/lines update or clean up correctly

7. General Plugin Lifecycle

Spot-check across multiple plugins:

  • plugin can be opened once
  • opening the same plugin again does not create a broken duplicate
  • plugin can be closed cleanly
  • plugin can be reopened after closing
  • active plugin registry stays consistent after open/close cycles

8. Final Sanity Check

Before considering the PR manually verified:

  • no startup warnings/errors from the refactored plugin stack
  • no obvious regression in imviewer interaction behavior
  • no obvious regression in app settings behavior
  • no obvious regression in plugin open/close behavior
  • at least one modal preview plugin and one live plugin have been tested successfully

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant