fix: Several new worklist issues - #6130
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Viewers
|
||||||||||||||||||||||||||||
| Project |
Viewers
|
| Branch Review |
fix/new-worklist-issues
|
| Run status |
|
| Run duration | 01m 43s |
| Commit |
|
| Committer | Bill Wallace |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
28
|
| View all changes introduced in this branch ↗︎ | |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds a customizable study-row double-click path for the default WorkList, including command registration, route wiring, StudyList behavior updates, and documentation. Also changes InputMultiSelect filtering so prefix matches are ordered before substring matches. ChangesCustom study double-click behavior
Estimated code review effort: 4 (Complex) | ~45 minutes InputMultiSelect Ranking Fix
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TableContent
participant WorkList
participant CommandsManager
participant LaunchDefaultMode
User->>TableContent: double-click study row
TableContent->>WorkList: onStudyDoubleClick(study, {defaultWorkflow, workflows})
WorkList->>CommandsManager: run(workList.onStudyDoubleClick, options)
CommandsManager->>LaunchDefaultMode: launchDefaultMode(study, workflows, defaultWorkflow, workflowId?)
LaunchDefaultMode->>LaunchDefaultMode: select workflow and launchWithStudy(study)
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
You mentioned that you prefer a command instead of a function, why did you choose not to implement a custom command for double click? |
|
From claude:
|
jbocce
left a comment
There was a problem hiding this comment.
Sorry for perhaps dropping some of these in the implementation although I am not sure they were explicitly mentioned in the requirements.
See my comments.
Because the modes don't know how to register command handlers, so the timing is wrong for anything to be available. |
…barrel Addresses PR review feedback: the double-click handler signature was written out in both TableProps and the WorkList customization cast, so the two could drift apart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Modes can now export getCommandsModule on their definition; appInit registers it (via the new ExtensionManager.registerCommandsModule) before the mode is instantiated, in a new 'WORKLIST' commands context, so the commands are available on the worklist before any mode route is entered. - The workList.onStudyDoubleClick customization is now a command run input (name/options) instead of a bare function, defaulting to the new launchDefaultMode command, which launches the default workflow falling back to the first applicable one. commandOptions.workflowId overrides it to a specific mode. - Duplicate mode ids are now skipped before running their modeFactory rather than after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@jbocce - I updated the PR to address your concerns and also added the command registration in modes so that it becomes possible to configure a customization as a command name instead of a function. That makes it cleaner overall. |
jbocce
left a comment
There was a problem hiding this comment.
One minor comment to consider, but approved!
* fix: Several new worklist issues * refactor: Export a single OnStudyDoubleClick type from the StudyList barrel Addresses PR review feedback: the double-click handler signature was written out in both TableProps and the WorkList customization cast, so the two could drift apart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: Run worklist study double-click as a command, registrable by modes - Modes can now export getCommandsModule on their definition; appInit registers it (via the new ExtensionManager.registerCommandsModule) before the mode is instantiated, in a new 'WORKLIST' commands context, so the commands are available on the worklist before any mode route is entered. - The workList.onStudyDoubleClick customization is now a command run input (name/options) instead of a bare function, defaulting to the new launchDefaultMode command, which launches the default workflow falling back to the first applicable one. commandOptions.workflowId overrides it to a specific mode. - Duplicate mode ids are now skipped before running their modeFactory rather than after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Context
The previous worklist preferred prefix-matching inputs for the modalities over other matches. That is almost always what a user wants so that if they type "PT" and hit enter, they DONT get "CTPT" as the value selected.
As well, added double-click behaviour so that the row doesn't disappear on a double click, and instead launches the default instance. I believe that had been agreed on but didn't get implemented. Made that customizeable, although I'd prefer to allow a cusotmization that specifies a command instead of a function.
Changes & Results
Testing
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
Summary by CodeRabbit
workList.onStudyDoubleClickcustomization to control what happens when double-clicking a study row in the default Work List.launchDefaultMode) for double-click behavior.workList.onStudyDoubleClickoption and default configuration.