fix: rework inconsistencies in split button implementation(#3086)
* feat: Combine the layout grid selector tool with the toggle MPR button into a split button. The primary button will always be the layout grid selector and the MPR toggle is in the drop down menu. This is the first step to later adding other (e.g. 3D) layout HPs to the menu. Summary of changes/fixes: - SplitButton now allows for its primary button to be any button component specified by a UI type. - SplitButton passes a className to its primary button component to dictate how it should be rendered based on its active/toggle status, hovering, menu visibility etc. - The default menu item renderer for the SplitButton, renders acitve toggle items correctly. - Items/buttons in the toolbar are now all the correct and same size. - Added ToolbarService.getButtonComponentForUIType to fetch a button component for a given UI type. - Spacing between toolbar items is now done at the Toolbar level to avoid double spacing and to ensure uniform spacing. - The drop down menu closes when clicking the primary button. - Show a mouse down click effect when clicking the primary button. Items to investigate still: The following are all slightly contrary to the design specs of the UI: - Is the click effect on the primary button satisfactory because with the hover border effect, the button appears slightly smaller during the mouse down/up operation? (MEDIUM) - On hover over the drop down button, the right-hand-side corners of the primary button appear rounded and NOT square. Also the hover over the drop down button border effects make the primary button appear smaller. (MEDIUM) - Should there be hover effects when hovering over either tooltip of the SplitButton? (LOW) * feat: Combine the layout grid selector tool with the toggle MPR button into a split button. Fixed broken e2e tests. data-cy and data-tool properties now passed down to primary button component. * feat: Combine the layout grid selector tool with the toggle MPR button into a split button. On drop down button hover, the primary button now stretches all the way right with no right border and with square corners in the top and bottom right as per the toolbar spec. The mouse down effect gives a full button look (i.e. no borders) with rounded corners all around and black text icon. * feat: Combine the layout grid selector tool with the toggle MPR button into a split button. Documented that the primary button of a split button can specify a UI type. * feat: Combine the layout grid selector tool with the toggle MPR button into a split button. Feedback from PR review to fix prettier formatting issue. * feat: Combine the layout grid selector tool with the toggle MPR button into a split button. Added TODOs as per Bill's request in the PR. * feat: Combine the layout grid selector tool with the toggle MPR button into a split button. Feedback from Alireza: reverted the layout selector and MPR tools to be separate toolbar items. Co-authored-by: Joe Boccanfuso <joe.boccanfuso@radicalimaging.com>
This commit is contained in:
Joe Boccanfuso
GitHub
parent
fb2a60ae19
commit
9b0d542526
@ -45,6 +45,10 @@ export default function toggleMPRHangingProtocol({
|
||||
ToolBarService,
|
||||
} = servicesManager.services;
|
||||
|
||||
// TODO Introduce a service to persist the state of the current hanging protocol/app.
|
||||
// So all of the code to persist the state here will no longer be needed. Perhaps
|
||||
// just the id of the current hanging protocol to toggle MPR off is needed.
|
||||
|
||||
const {
|
||||
activeViewportIndex,
|
||||
viewports,
|
||||
|
||||
@ -1,4 +1,5 @@
|
||||
import React, { useEffect, useState } from 'react';
|
||||
import classnames from 'classnames';
|
||||
|
||||
export default function Toolbar({ servicesManager }) {
|
||||
const { ToolBarService } = servicesManager.services;
|
||||
@ -46,15 +47,18 @@ export default function Toolbar({ servicesManager }) {
|
||||
// These can... Trigger toolbar events based on updates?
|
||||
// Then sync using useEffect, or simply modify the state here?
|
||||
return (
|
||||
<Component
|
||||
key={id}
|
||||
id={id}
|
||||
{...componentProps}
|
||||
bState={buttonState}
|
||||
isActive={isActive}
|
||||
onInteraction={args => ToolBarService.recordInteraction(args)}
|
||||
servicesManager={servicesManager}
|
||||
/>
|
||||
// The margin for separating the tools on the toolbar should go here and NOT in each individual component (button) item.
|
||||
// This allows for the individual items to be included in other UI components where perhaps alternative margins are desired.
|
||||
<div key={id} className={classnames('mr-1')}>
|
||||
<Component
|
||||
id={id}
|
||||
{...componentProps}
|
||||
bState={buttonState}
|
||||
isActive={isActive}
|
||||
onInteraction={args => ToolBarService.recordInteraction(args)}
|
||||
servicesManager={servicesManager}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
})}
|
||||
</>
|
||||
|
||||
@ -6,12 +6,18 @@ import {
|
||||
useViewportGrid,
|
||||
} from '@ohif/ui';
|
||||
|
||||
function LayoutSelector({ rows, columns, servicesManager }) {
|
||||
function LayoutSelector({
|
||||
rows,
|
||||
columns,
|
||||
className,
|
||||
servicesManager,
|
||||
...rest
|
||||
}) {
|
||||
const [isOpen, setIsOpen] = useState(false);
|
||||
const [disableSelector, setDisableSelector] = useState(false);
|
||||
const [viewportGridState, viewportGridService] = useViewportGrid();
|
||||
|
||||
const { HangingProtocolService } = servicesManager.services;
|
||||
const { HangingProtocolService, ToolBarService } = servicesManager.services;
|
||||
|
||||
const closeOnOutsideClick = () => {
|
||||
if (isOpen) {
|
||||
@ -24,12 +30,6 @@ function LayoutSelector({ rows, columns, servicesManager }) {
|
||||
HangingProtocolService.EVENTS.PROTOCOL_CHANGED,
|
||||
evt => {
|
||||
const { protocol } = evt;
|
||||
|
||||
if (protocol.id === 'mpr') {
|
||||
setDisableSelector(true);
|
||||
} else {
|
||||
setDisableSelector(false);
|
||||
}
|
||||
}
|
||||
);
|
||||
|
||||
@ -55,20 +55,42 @@ function LayoutSelector({ rows, columns, servicesManager }) {
|
||||
const onInteractionHandler = () => setIsOpen(!isOpen);
|
||||
const DropdownContent = isOpen ? OHIFLayoutSelector : null;
|
||||
|
||||
const onSelectionHandler = ({ numRows, numCols }) => {
|
||||
// TODO Introduce a service to persist the state of the current hanging protocol/app.
|
||||
|
||||
// TODO Here the layout change will amount to a change of hanging protocol as specified by the extension for this layout selector tool
|
||||
// followed by the change of the grid itself.
|
||||
if (HangingProtocolService.getActiveProtocol().protocol.id === 'mpr') {
|
||||
ToolBarService.recordInteraction({
|
||||
groupId: 'MPR',
|
||||
itemId: 'MPR',
|
||||
interactionType: 'toggle',
|
||||
commands: [
|
||||
{
|
||||
commandName: 'toggleMPR',
|
||||
commandOptions: {},
|
||||
context: 'CORNERSTONE',
|
||||
},
|
||||
],
|
||||
});
|
||||
}
|
||||
viewportGridService.setLayout({ numRows, numCols });
|
||||
};
|
||||
|
||||
return (
|
||||
<ToolbarButton
|
||||
id="Layout"
|
||||
label="Grid Layout"
|
||||
icon="tool-layout"
|
||||
onInteraction={onInteractionHandler}
|
||||
className={className}
|
||||
rounded={rest.rounded}
|
||||
dropdownContent={
|
||||
DropdownContent !== null && (
|
||||
<DropdownContent
|
||||
rows={rows}
|
||||
columns={columns}
|
||||
onSelection={({ numRows, numCols }) => {
|
||||
viewportGridService.setLayout({ numRows, numCols });
|
||||
}}
|
||||
onSelection={onSelectionHandler}
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
@ -35,27 +35,7 @@ export default function getToolbarModule({ commandsManager, servicesManager }) {
|
||||
{
|
||||
name: 'ohif.toggle',
|
||||
defaultComponent: ToolbarButton,
|
||||
requiredConfig: [],
|
||||
optionalConfig: [],
|
||||
requiredProps: [],
|
||||
optionalProps: [],
|
||||
clickHandler: (evt, clickedBtn, btnSectionName) => {
|
||||
const { props } = clickedBtn;
|
||||
const allButtons = toolbarService.getButtons();
|
||||
const thisButton = allButtons[clickedBtn.id];
|
||||
|
||||
// Set our clicked button to active
|
||||
thisButton.props.isActive = !thisButton.props.isActive;
|
||||
|
||||
// Run button logic/command
|
||||
commandsManager.runCommand(props.commandName, props.commandOptions);
|
||||
|
||||
// What if just toggled "content"?
|
||||
// commandName OR content?
|
||||
|
||||
// Set buttons & trigger notification
|
||||
toolbarService.setButtons(allButtons);
|
||||
},
|
||||
clickHandler: () => {},
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
@ -15,7 +15,7 @@ const { windowLevelPresets } = defaults;
|
||||
* @param {*} icon
|
||||
* @param {*} label
|
||||
*/
|
||||
function _createButton(type, id, icon, label, commands, tooltip) {
|
||||
function _createButton(type, id, icon, label, commands, tooltip, uiType) {
|
||||
return {
|
||||
id,
|
||||
icon,
|
||||
@ -23,6 +23,7 @@ function _createButton(type, id, icon, label, commands, tooltip) {
|
||||
type,
|
||||
commands,
|
||||
tooltip,
|
||||
uiType,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@ -287,4 +287,10 @@ export default class ToolBarService {
|
||||
componentProps: Object.assign({}, btn.props, props),
|
||||
};
|
||||
}
|
||||
|
||||
getButtonComponentForUIType(uiType: string) {
|
||||
return uiType
|
||||
? this._buttonTypes()[uiType]?.defaultComponent ?? null
|
||||
: null;
|
||||
}
|
||||
}
|
||||
|
||||
@ -44,6 +44,11 @@ export default function getToolbarModule({ commandsManager, servicesManager }) {
|
||||
defaultComponent: ToolbarLayoutSelector,
|
||||
clickHandler: (evt, clickedBtn, btnSectionName) => {},
|
||||
},
|
||||
{
|
||||
name: 'ohif.toggle',
|
||||
defaultComponent: ToolbarButton,
|
||||
clickHandler: () => {},
|
||||
},
|
||||
];
|
||||
}
|
||||
```
|
||||
@ -129,8 +134,11 @@ There are three main types of toolbar buttons:
|
||||
You can use the `ohif.splitButton` type to build a button with extra tools in
|
||||
the dropdown.
|
||||
|
||||
- First you need to give your `primary` tool definition to the split button
|
||||
- the `secondary` properties can be a simple arrow down (`chevron-down` icon)
|
||||
- First you need to give your `primary` tool definition to the split button. The primary
|
||||
tool can specify a `uiType` property which can be one of the button types returned by
|
||||
`getToolbarModule` that is a variation of `ToolbarButton`. If `uiType` is omitted then
|
||||
`ToolbarButton` is used by default.
|
||||
- The `secondary` properties can be a simple arrow down (`chevron-down` icon)
|
||||
- For adding the extra tools add them to the `items` list.
|
||||
|
||||
You can see below how `longitudinal` mode is using the available toolbarModule
|
||||
|
||||
@ -140,8 +140,8 @@ const IconButton = ({
|
||||
ref={buttonElement}
|
||||
onClick={handleOnClick}
|
||||
type={type}
|
||||
data-cy={id}
|
||||
{...rest}
|
||||
data-cy={rest['data-cy'] ?? id}
|
||||
data-tool={rest['data-tool']}
|
||||
>
|
||||
{React.cloneElement(children, {
|
||||
className: classnames(iconSizeClasses[size], 'fill-current'),
|
||||
|
||||
@ -4,16 +4,17 @@ import classNames from 'classnames';
|
||||
import OutsideClickHandler from 'react-outside-click-handler';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
|
||||
import { Icon, Tooltip, ListMenu } from '../';
|
||||
import { Icon, Tooltip, ListMenu, ToolbarButton } from '../';
|
||||
|
||||
const baseClasses = {
|
||||
Button:
|
||||
'flex items-center rounded-md border-transparent border-2 cursor-pointer group/button',
|
||||
'flex items-center rounded-md border-transparent cursor-pointer group/button',
|
||||
Primary:
|
||||
'h-full flex flex-1 items-center rounded-md rounded-tr-none rounded-br-none group/primary',
|
||||
// By default border on left, top and bottom for hover effect and only rounded on left side.
|
||||
// Extra padding on right to componensate for no right border.
|
||||
'h-full border-l-2 border-t-2 border-b-2 rounded-tl-md rounded-bl-md group/primary !pl-2 !py-2',
|
||||
Secondary:
|
||||
'h-full flex items-center justify-center rounded-tr-md rounded-br-md w-4 group/secondary',
|
||||
PrimaryIcon: 'w-5 h-5',
|
||||
'h-full flex items-center justify-center rounded-tr-md rounded-br-md w-4 border-2 border-transparent group/secondary',
|
||||
SecondaryIcon: 'w-4 h-full stroke-1',
|
||||
Separator: 'border-l py-2.5',
|
||||
Content: 'absolute z-10 top-0 mt-12',
|
||||
@ -31,29 +32,31 @@ const classes = {
|
||||
Primary: ({ primary, isExpanded }) =>
|
||||
classNames(
|
||||
baseClasses.Primary,
|
||||
primary.isActive && !isExpanded
|
||||
? 'bg-primary-light rounded-tr-md rounded-br-md active'
|
||||
: isExpanded
|
||||
? 'bg-primary-dark'
|
||||
: 'bg-secondary-dark hover:bg-primary-dark'
|
||||
primary.isActive
|
||||
? isExpanded
|
||||
? 'border-primary-dark !bg-primary-dark hover:border-primary-dark text-primary-light'
|
||||
: `${
|
||||
primary.isToggle
|
||||
? 'border-secondary-dark bg-secondary-light'
|
||||
: 'border-primary-light bg-primary-light'
|
||||
}
|
||||
border-2 rounded-md !p-2` // Full, rounded border with less right padding when active.
|
||||
: `focus:!text-black focus:!rounded-md focus:!border-primary-light focus:!bg-primary-light
|
||||
${
|
||||
isExpanded
|
||||
? 'border-primary-dark bg-primary-dark !text-primary-light'
|
||||
: 'border-secondary-dark bg-secondary-dark group-hover/button:border-primary-dark group-hover/button:text-primary-light hover:bg-primary-dark hover:border-primary-dark focus:!text-black'
|
||||
}
|
||||
`
|
||||
),
|
||||
Secondary: ({ isExpanded, primary }) =>
|
||||
classNames(
|
||||
baseClasses.Secondary,
|
||||
isExpanded
|
||||
? 'bg-primary-light rounded-tr-md rounded-br-md'
|
||||
? 'bg-primary-light !rounded-tr-md !rounded-br-md'
|
||||
: primary.isActive
|
||||
? 'bg-secondary-dark'
|
||||
: 'hover:bg-primary-dark bg-secondary-dark'
|
||||
),
|
||||
PrimaryIcon: ({ primary, isExpanded }) =>
|
||||
classNames(
|
||||
baseClasses.PrimaryIcon,
|
||||
!primary.isActive &&
|
||||
'group-hover/primary:text-primary-light group-hover/secondary:text-primary-light group-hover/button:text-primary-light',
|
||||
primary.isActive && !isExpanded
|
||||
? 'text-primary-dark '
|
||||
: 'text-common-bright'
|
||||
: 'hover:bg-primary-dark bg-secondary-dark group-hover/button:border-primary-dark'
|
||||
),
|
||||
SecondaryIcon: ({ isExpanded }) =>
|
||||
classNames(
|
||||
@ -85,9 +88,12 @@ const SplitButton = ({
|
||||
items: _items,
|
||||
renderer,
|
||||
onInteraction,
|
||||
servicesManager,
|
||||
}) => {
|
||||
const { t } = useTranslation('Buttons');
|
||||
|
||||
const { ToolBarService } = servicesManager.services;
|
||||
|
||||
const { primaryToolId, toggles } = bState;
|
||||
/* Bubbles up individual item clicks */
|
||||
const getSplitButtonItems = items =>
|
||||
@ -107,7 +113,7 @@ const SplitButton = ({
|
||||
|
||||
setState(state => ({
|
||||
...state,
|
||||
primary: !isAction ? { ...item, index } : state.primary,
|
||||
primary: !isAction && isRadio ? { ...item, index } : state.primary,
|
||||
isExpanded: false,
|
||||
items: getSplitButtonItems(_items).filter(item =>
|
||||
isRadio && !isAction ? item.index !== index : true
|
||||
@ -133,21 +139,44 @@ const SplitButton = ({
|
||||
setState(state => ({ ...state, isHovering: false }));
|
||||
const outsideClickHandler = () =>
|
||||
setState(state => ({ ...state, isExpanded: false }));
|
||||
const onPrimaryClickHandler = () => {
|
||||
onInteraction({
|
||||
groupId,
|
||||
itemId: state.primary.id,
|
||||
interactionType: state.primary.type,
|
||||
// splitButtonId? (so we can track group?)
|
||||
// info to fire item's command/event?
|
||||
//
|
||||
commands: state.primary.commands,
|
||||
});
|
||||
};
|
||||
|
||||
const isPrimaryToggle = state.primary.type === 'toggle';
|
||||
const isPrimaryActive =
|
||||
(state.primary.type === 'tool' && primaryToolId === state.primary.id) ||
|
||||
(state.primary.type === 'toggle' && toggles[state.primary.id] === true);
|
||||
(isPrimaryToggle && toggles[state.primary.id] === true);
|
||||
|
||||
const PrimaryButtonComponent =
|
||||
ToolBarService.getButtonComponentForUIType(state.primary.uiType) ??
|
||||
ToolbarButton;
|
||||
|
||||
const primaryButtonClassName = classes.Primary({
|
||||
...state,
|
||||
primary: { isActive: isPrimaryActive, isToggle: isPrimaryToggle },
|
||||
});
|
||||
|
||||
const DefaultListItemRenderer = ({ type, icon, label, t, id }) => {
|
||||
const isActive = type === 'toggle' && toggles[id] === true;
|
||||
|
||||
return (
|
||||
<div
|
||||
className={classNames(
|
||||
'flex flex-row items-center p-3 h-8 w-full hover:bg-primary-dark',
|
||||
'text-base whitespace-pre',
|
||||
isActive && 'bg-primary-dark',
|
||||
isActive
|
||||
? 'text-[#348CFD]'
|
||||
: 'text-common-bright hover:bg-primary-dark hover:text-primary-light'
|
||||
)}
|
||||
>
|
||||
<span className="mr-4">
|
||||
<Icon name={icon} className="w-5 h-5" />
|
||||
</span>
|
||||
<span className="mr-5">{t(label)}</span>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
const listItemRenderer = renderer || DefaultListItemRenderer;
|
||||
|
||||
return (
|
||||
<OutsideClickHandler onOutsideClick={outsideClickHandler}>
|
||||
@ -162,32 +191,20 @@ const SplitButton = ({
|
||||
onMouseLeave={onMouseLeaveHandler}
|
||||
>
|
||||
<div className={classes.Interface}>
|
||||
<div
|
||||
onClick={onPrimaryClickHandler}
|
||||
className={classes.Primary({
|
||||
...state,
|
||||
primary: { isActive: isPrimaryActive },
|
||||
})}
|
||||
data-tool={state.primary.id}
|
||||
data-cy={`${groupId}-split-button-primary`}
|
||||
>
|
||||
<Tooltip
|
||||
isDisabled={!state.primary.tooltip}
|
||||
content={state.primary.tooltip}
|
||||
>
|
||||
<div
|
||||
className="flex items-center justify-center w-full h-full"
|
||||
style={{ padding: '10px' }}
|
||||
>
|
||||
<Icon
|
||||
name={state.primary.icon}
|
||||
className={classes.PrimaryIcon({
|
||||
...state,
|
||||
primary: { isActive: isPrimaryActive },
|
||||
})}
|
||||
/>
|
||||
</div>
|
||||
</Tooltip>
|
||||
<div onClick={outsideClickHandler}>
|
||||
<PrimaryButtonComponent
|
||||
key={state.primary.id}
|
||||
{...state.primary}
|
||||
bState={bState}
|
||||
isActive={isPrimaryActive}
|
||||
onInteraction={args => ToolBarService.recordInteraction(args)}
|
||||
servicesManager={servicesManager}
|
||||
// All rounding is taken care of by className
|
||||
rounded="none"
|
||||
className={primaryButtonClassName}
|
||||
data-tool={state.primary.id}
|
||||
data-cy={`${groupId}-split-button-primary`}
|
||||
/>
|
||||
</div>
|
||||
<div
|
||||
className={classes.Separator({
|
||||
@ -226,7 +243,8 @@ const SplitButton = ({
|
||||
>
|
||||
<ListMenu
|
||||
items={state.items}
|
||||
renderer={args => renderer({ ...args, t })}
|
||||
bState={bState}
|
||||
renderer={args => listItemRenderer({ ...args, t })}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
@ -234,24 +252,6 @@ const SplitButton = ({
|
||||
);
|
||||
};
|
||||
|
||||
const DefaultListItemRenderer = ({ icon, label, isActive, t }) => (
|
||||
<div
|
||||
className={classNames(
|
||||
'flex flex-row items-center p-3 h-8 w-full hover:bg-primary-dark',
|
||||
isActive && 'bg-primary-dark'
|
||||
)}
|
||||
>
|
||||
<span className="mr-4 text-base whitespace-pre text-common-bright">
|
||||
<Icon name={icon} className="w-5 h-5 text-common-bright" />
|
||||
</span>
|
||||
<span className="mr-5 text-base whitespace-pre text-common-bright">
|
||||
{t(label)}
|
||||
</span>
|
||||
</div>
|
||||
);
|
||||
|
||||
const noop = () => {};
|
||||
|
||||
SplitButton.defaultProps = {
|
||||
isRadio: false,
|
||||
isAction: false,
|
||||
@ -266,7 +266,7 @@ SplitButton.defaultProps = {
|
||||
tooltip: 'More Measure Tools',
|
||||
},
|
||||
items: [],
|
||||
renderer: DefaultListItemRenderer,
|
||||
renderer: null,
|
||||
};
|
||||
|
||||
SplitButton.propTypes = {
|
||||
|
||||
@ -14,7 +14,9 @@ const ToolbarButton = ({
|
||||
dropdownContent,
|
||||
//
|
||||
isActive: _isActive,
|
||||
className,
|
||||
bState = {},
|
||||
...rest
|
||||
//
|
||||
}) => {
|
||||
const { primaryToolId } = bState;
|
||||
@ -49,7 +51,7 @@ const ToolbarButton = ({
|
||||
variant={isActive ? 'contained' : 'text'}
|
||||
bgColor={bgClasses[type]}
|
||||
size="toolbar"
|
||||
className={classnames('mx-1', activeClass, classes[type])}
|
||||
className={classnames(activeClass, classes[type], className)}
|
||||
onClick={() => {
|
||||
onInteraction({
|
||||
itemId: id,
|
||||
@ -60,6 +62,7 @@ const ToolbarButton = ({
|
||||
name={label}
|
||||
key={id}
|
||||
id={id}
|
||||
{...rest}
|
||||
>
|
||||
<Icon name={icon} />
|
||||
</IconButton>
|
||||
|
||||
Loading…
Reference in New Issue
Block a user