-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ui(components): Adds ToolButton core components and toolbar updates #4728
Open
dan-rukas
wants to merge
28
commits into
OHIF:master
Choose a base branch
from
dan-rukas:feat/toolbutton-components
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Design will only be handled by any ui-next components
order based on what's defined in the tool: label, tooltip, id
✅ Deploy Preview for ohif-dev canceled.
|
❌ Deploy Preview for ohif-platform-docs failed. Why did it fail? →
|
See previous pull request for additional commit history for all these changes. All working updates are included in this latest PR. This is only for additional history context: #4670 |
…roup, update toolbar button group references, and improve disabled state styling. Cleaned up imports and removed legacy components for better maintainability.
…etter dropdown item separation, refactored interaction handling in ToolButtonListWrapper, and updated imports across the platform. Improved accessibility by ensuring missing icons are handled gracefully. fixed disabled button and tooltip Enhance UI components: Added className support to ToolButtonListWrapper, updated icon styles in TrackedCornerstoneViewport and Thumbnail, and adjusted dimensions for ToolWindowRegion icon. Improved toolbar button organization by uncommenting relevant tools in longitudinal mode. Refactor toolbar components: Introduced size support for ToolButton, updated ToolbarButtonGroupWithServices to utilize new ToolButton, and removed the obsolete ToolButtonGroup component. Enhanced toolbar organization by adjusting component imports and ensuring legacy components are clearly marked. Improved overall maintainability and consistency in button styling. wip Implement ToolboxProvider and useToolbox hook: Added a new context for managing toolbox state and actions, enabling better organization of toolbar tools and options. Updated relevant imports across the application to integrate the new toolbox functionality. Refactor Toolbox imports: Updated Toolbox imports across multiple extensions to use '@ohif/extension-default' instead of '@ohif/ui-next'. Introduced a new Toolbox component in the default extension for better organization and functionality. Cleaned up unused Toolbox references in the UI components. Refactor toolbar components: Introduced ToolButtonGroupWrapper for enhanced button grouping functionality, updated legacy toolbar component imports to their new counterparts, and adjusted toolbar button section creation in segmentation mode. Improved organization and maintainability of toolbar components. Refactor toolbar components: Removed legacy className properties and introduced new ToolBox components for better organization. Updated toolbar button evaluation logic to enhance functionality and maintainability. Cleaned up unused imports and improved button state handling across multiple extensions. Refactor toolbarButtons: Removed unused buttons (Probe, Angle, RectangleROI, CalibrationLine) to streamline the toolbar. This cleanup enhances maintainability and focuses on essential tools for segmentation mode.
- Moved `getToggledClassName` to a new utility function in `ui-next` - Updated imports in cornerstone and default extensions to use the new utility - Added `utils` export to `ui-next` index - Removed local implementations of `getToggledClassName` - Minor improvements to type safety in `usePatientInfo` hook
…d state handling - Added `disabledText` prop to support more informative disabled button states - Implemented tooltip rendering for both primary and secondary toolbar buttons - Updated ToolbarService to propagate `disabledText` through button evaluation - Refactored ToolButtonListWrapper and ToolButtonListItem to use spread props and support new tooltip functionality - Simplified component prop handling by using object spread
- Modified `getToggledClassName` utility to directly return a string className - Updated `evaluate.cine` in toolbar module to wrap returned className in an object - Maintains consistent styling logic while simplifying the utility function
Remove tooltip for list items to address early tooltip display issue in dropdown menu, maintaining tooltip only for disabled items with disabledText
Update DropdownMenuContent to align at the start with a negative offset, improving dropdown menu positioning in the ToolButtonList component
- Updated toolbar buttons to use new toolbox button types in TMTV mode - Added `disabledText` prop to ToolbarButton for improved disabled state handling - Removed debug statements from ToolBoxWrapper - Adjusted styling for SplitButton and toolbar interactions - Introduced new toolbox size and button group configurations
- Updated ToolButtonListDivider to conditionally hide/show based on primary button state - Adjusted DropdownMenuContent alignOffset from -35 to -40 for improved positioning
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
ToolButton
,ToolButtonSmall
, andToolButtonList
inui-next
for single‐tool and “split button” functionality within the OHIF toolbar and smaller tools in panels.ToolbarService
logic.ui-next
components for newOHIFToolSettings
to replace legacy Inputs, Sliders, and text in Segmentation Tool optionstoolbarService
. Design styles are located in newToolButton
components. The panel and main toolbar no longer track separate active states, eliminating the “stuck” tool issue. Separated “which tool is active” (controlled bytoolbarService
) from “tool configuration” (stored inuseToolbox
).Changes & Results
ui-next: new ToolButton core components
ToolButton
(new component) - Handles single-tool activation with a tooltip and icon.Button
andTooltip
from ui-nextToolButtonList
(new component) - ToolButton with secondary tools.ToolButton
andDropdown
from ui-nextToolButtonSmall
(new component) - Grouped and Single Tool Buttons for panel use.ToolButton
from ui-nextui-next: OHIFToolSettings component group
RowInputRange
(new component) - UsesSlider
,Input
,Label
from ui-nextRowSegmentedControl
(new component) - UsesLabel
andTabs
from ui-nextRowDoubleRange
(new component) - UsesDoubleSlider
from ui-nextui-next: OHIFToolbox changes
Toolbox
- Changes:toolbarService
events to detect when the globally active tool changes. Ensures that when the user selects a different tool in the main toolbar, the panel sees that change and updates its local “activeTool” if necessary.handleToolSelect
), it now callscommandsManager.runCommand('setToolActiveToolbar', { toolName: id })
. This sets the active tool globally for the entire application, rather than only locally in the panel. It keeps the panel and main toolbar in sync.useEffect
that runs whenever toolbar buttons or local tool options change. Ensures that each tool’s “advanced settings” (like range sliders) in the panel are properly merged/initialized with the global tool definitions.ToolboxUI
- Changes:const isActive = activeTool === id;
and replaced them withtoolDef.isActive
. Now relies on the globaltoolbarService
to flag whether a tool is active, allowing the panel to match the main toolbar’s state.<>...</>
with inline comments. Using<React.Fragment>
fixes those build errors.App file changes
getToolbarModule.ts
(and similar “evaluate” modules) - Confirmed thatevaluate
functions properly returnisActive: true
for whichever tool is primary in a given viewport.toolbarService
calls these “evaluate” functions to mark tools as disabled or active. The panel’s buttons now display correctly since they read the sameisActive
that is set here.evaluate
was already returning the correctisActive
states. We only ensured that these modules align with the global “active tool” logic. Main toolbar and panel buttons see the same final “active” or “disabled” states.segmentationButtons.ts
radioGroup
legacy button to work withbuttonGroup
.Testing
New Toolbar components are currently working in
Basic Viewer
andSegmenation
modes (tested in Chrome)Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment