Skip to content

fix(segmentation): apply modeConfiguration overrides#6051

Open
igoroctaviano wants to merge 1 commit into
OHIF:masterfrom
igoroctaviano:fix/segmentation-mode-configuration
Open

fix(segmentation): apply modeConfiguration overrides#6051
igoroctaviano wants to merge 1 commit into
OHIF:masterfrom
igoroctaviano:fix/segmentation-mode-configuration

Conversation

@igoroctaviano
Copy link
Copy Markdown
Contributor

@igoroctaviano igoroctaviano commented May 29, 2026

Summary

The segmentation mode's modeFactory accepts a modeConfiguration argument but never applies it to the returned mode instance. As a result, any modesConfiguration['@ohif/mode-segmentation'] overrides from the app config (such as hide, displayName, routeName, etc.) are silently ignored for this mode.

Other modes (e.g. usAnnotation, tmtv, microscopy, preclinical-4d) already spread ...modeConfiguration into their returned object. This change brings the segmentation mode in line with them.

// platform/app/src/appInit.js
const modeConfiguration =
  appConfig.modesConfiguration && appConfig.modesConfiguration[id]
    ? appConfig.modesConfiguration[id]
    : {};
mode = await mode.modeFactory({ modeConfiguration, loadModules });

Before this fix, e.g. the following config had no effect on the segmentation mode:

modesConfiguration: {
  '@ohif/mode-segmentation': { hide: true },
},

Changes

  • Spread ...modeConfiguration into the object returned by the segmentation modeFactory.

Test plan

  • Add modesConfiguration: { '@ohif/mode-segmentation': { hide: true } } to the app config and confirm the Segmentation mode no longer appears in the mode selector.
  • Confirm overriding displayName for @ohif/mode-segmentation is reflected in the mode selector.
  • Confirm default behavior (no override) is unchanged.

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes
    • Fixed mode configuration to ensure all provided settings are properly applied when creating mode definitions.

Review Change Stack

Greptile Summary

This one-line fix ensures that modeConfiguration overrides passed via appConfig.modesConfiguration['@ohif/mode-segmentation'] are actually applied to the segmentation mode instance. Previously the modeFactory accepted the argument but never used it, silently discarding any app-level overrides such as hide, displayName, or routeName.

  • Spreads ...modeConfiguration at the end of the object returned by modeFactory, giving app-config overrides precedence over defaults — identical to the pattern already used in every other OHIF mode.

Confidence Score: 5/5

Safe to merge — single-line additive change that aligns the segmentation mode with all other modes in the codebase.

The change is a one-line spread of a well-typed, null-safe argument that has already been defaulted to {} before being passed in. It touches no logic, no service wiring, and no lifecycle functions. The same pattern is proven across every other mode in the repo.

No files require special attention.

Important Files Changed

Filename Overview
modes/segmentation/src/index.tsx Adds ...modeConfiguration spread at the end of the modeFactory return object, consistent with how all other modes (usAnnotation, tmtv, microscopy, preclinical-4d) already handle this argument.

Reviews (1): Last reviewed commit: "fix(segmentation): apply modeConfigurati..." | Re-trigger Greptile

The segmentation mode's modeFactory accepted a modeConfiguration
argument but never applied it to the returned mode instance, so
modesConfiguration overrides from the app config (e.g. hide,
displayName) were silently ignored for this mode. Spread
modeConfiguration into the returned object to match the other modes.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 29, 2026

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit 4ea8c94
🔍 Latest deploy log https://app.netlify.com/projects/ohif-dev/deploys/6a19d09623e88e0008babc15
😎 Deploy Preview https://deploy-preview-6051--ohif-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

📝 Walkthrough

Walkthrough

The modeFactory function in the segmentation mode now propagates configuration options into the returned mode definition by spreading modeConfiguration into the object literal, ensuring configuration fields are included in the mode definition rather than discarded.

Changes

Mode Configuration Propagation

Layer / File(s) Summary
modeFactory configuration spread
modes/segmentation/src/index.tsx
modeFactory now spreads modeConfiguration into the returned mode object, merging any provided configuration fields into the mode definition.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

A tiny thread of code is spun,
Configuration spread, now not undone,
The factory hops with glee so bright,
One little spread makes configs right! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(segmentation): apply modeConfiguration overrides' clearly and specifically describes the main change—spreading modeConfiguration into the segmentation mode's returned object.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description comprehensively covers all required template sections with clear context, detailed changes, and a concrete test plan.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

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