Skip to content

Commit

Permalink
Revert "Delay cascading menu closing after mouseout (#4645)"
Browse files Browse the repository at this point in the history
This reverts commit 17d7415.
  • Loading branch information
cmdcolin committed Nov 8, 2024
1 parent c5554a8 commit 9d34f8c
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 142 deletions.
46 changes: 13 additions & 33 deletions packages/core/ui/CascadingMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,11 @@ import {
} from 'material-ui-popup-state/hooks'
import HoverMenu from 'material-ui-popup-state/HoverMenu'
import ChevronRight from '@mui/icons-material/ChevronRight'
import { useDebounce } from '../util'

interface ContextType {
parentPopupState: PopupState | null
rootPopupState: PopupState | null
}
const CascadingContext = React.createContext<ContextType>({
const CascadingContext = React.createContext({
parentPopupState: null,
rootPopupState: null,
})
} as { parentPopupState: PopupState | null; rootPopupState: PopupState | null })

function CascadingMenuItem({
onClick,
Expand Down Expand Up @@ -70,6 +65,7 @@ function CascadingSubmenu({
title: React.ReactNode
onMenuItemClick: Function
Icon: React.ComponentType<SvgIconProps> | undefined

inset: boolean
menuItems: JBMenuItem[]
popupId: string
Expand All @@ -80,16 +76,6 @@ function CascadingSubmenu({
variant: 'popover',
parentPopupState,
})

// avoid the submenu closing instantly after mouse out
const isOpenDebounce = useDebounce(popupState.isOpen, 400)

// ternary reasoning: if the popup is toggled to false, is the debounce,
// otherwise it is true, and then don't debounce it (which would delay e.g.
// opening a menu from scratch)
const isOpenDebounceEffective =
// eslint-disable-next-line unicorn/prefer-logical-operator-over-ternary
!popupState.isOpen ? isOpenDebounce : popupState.isOpen
return (
<>
<MenuItem {...bindHover(popupState)} {...bindFocus(popupState)}>
Expand All @@ -103,18 +89,9 @@ function CascadingSubmenu({
</MenuItem>
<CascadingSubmenuHover
{...props}
anchorOrigin={{
vertical: 'top',
horizontal: 'right',
}}
transformOrigin={{
vertical: 'top',
horizontal: 'left',
}}
popupState={{
...popupState,
isOpen: isOpenDebounceEffective,
}}
anchorOrigin={{ vertical: 'top', horizontal: 'right' }}
transformOrigin={{ vertical: 'top', horizontal: 'left' }}
popupState={popupState}
/>
</>
)
Expand Down Expand Up @@ -201,6 +178,12 @@ function CascadingMenuList({
closeAfterItemClick: boolean
onMenuItemClick: Function
}) {
function handleClick(callback: Function) {
return (event: React.MouseEvent<HTMLLIElement>) => {
onMenuItemClick(event, callback)
}
}

const hasIcon = menuItems.some(m => 'icon' in m && m.icon)
return (
<>
Expand Down Expand Up @@ -238,10 +221,7 @@ function CascadingMenuList({
key={`${item.label}-${idx}`}
closeAfterItemClick={closeAfterItemClick}
onClick={
'onClick' in item
? (event: React.MouseEvent<HTMLLIElement>) =>
onMenuItemClick(event, item.onClick)
: undefined
'onClick' in item ? handleClick(item.onClick) : undefined
}
disabled={Boolean(item.disabled)}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import TrackLabelDragHandle from './TrackLabelDragHandle'

const useStyles = makeStyles()(theme => ({
root: {
zIndex: 1000,
background: alpha(theme.palette.background.paper, 0.8),
'&:hover': {
background: theme.palette.background.paper,
Expand Down Expand Up @@ -52,7 +51,66 @@ const TrackLabel = observer(
) {
const { classes, cx } = useStyles()
const view = getContainingView(track) as LGV
const session = getSession(track)
const trackConf = track.configuration
const minimized = track.minimized
const trackId = getConf(track, 'trackId')
const trackName = getTrackName(trackConf, session)
const items = [
{
label: 'Track order',
type: 'subMenu',
priority: 2000,
subMenu: [
{
label: minimized ? 'Restore track' : 'Minimize track',
icon: minimized ? AddIcon : MinimizeIcon,
onClick: () => {
track.setMinimized(!minimized)
},
},
...(view.tracks.length > 2
? [
{
label: 'Move track to top',
icon: KeyboardDoubleArrowUpIcon,
onClick: () => {
view.moveTrackToTop(track.id)
},
},
]
: []),

{
label: 'Move track up',
icon: KeyboardArrowUpIcon,
onClick: () => {
view.moveTrackUp(track.id)
},
},
{
label: 'Move track down',
icon: KeyboardArrowDownIcon,
onClick: () => {
view.moveTrackDown(track.id)
},
},
...(view.tracks.length > 2
? [
{
label: 'Move track to bottom',
icon: KeyboardDoubleArrowDownIcon,
onClick: () => {
view.moveTrackToBottom(track.id)
},
},
]
: []),
],
},
...(session.getTrackActionMenuItems?.(trackConf) || []),
...track.trackMenuItems(),
].sort((a, b) => (b?.priority || 0) - (a?.priority || 0))

return (
<Paper ref={ref} className={cx(className, classes.root)}>
Expand All @@ -64,113 +122,29 @@ const TrackLabel = observer(
>
<CloseIcon fontSize="small" />
</IconButton>
<TrackLabelTypography track={track} />
<TrackLabelMenu track={track} />

<Typography
variant="body1"
component="span"
className={classes.trackName}
onMouseDown={event => {
// avoid becoming a click-and-drag action on the lgv
event.stopPropagation()
}}
>
<SanitizedHTML
html={[trackName, minimized ? '(minimized)' : '']
.filter(f => !!f)
.join(' ')}
/>
</Typography>

<CascadingMenuButton menuItems={items} data-testid="track_menu_icon">
<MoreVertIcon fontSize="small" />
</CascadingMenuButton>
</Paper>
)
}),
)

const TrackLabelTypography = observer(function ({
track,
}: {
track: BaseTrackModel
}) {
const { classes } = useStyles()
const session = getSession(track)
const trackConf = track.configuration
const minimized = track.minimized
const trackName = getTrackName(trackConf, session)
return (
<Typography
variant="body1"
component="span"
className={classes.trackName}
onMouseDown={event => {
// avoid becoming a click-and-drag action on the lgv
event.stopPropagation()
}}
>
<SanitizedHTML
html={[trackName, minimized ? '(minimized)' : '']
.filter(f => !!f)
.join(' ')}
/>
</Typography>
)
})

const TrackLabelMenu = observer(function ({
track,
}: {
track: BaseTrackModel
}) {
const view = getContainingView(track) as LGV
const session = getSession(track)
const minimized = track.minimized
const trackConf = track.configuration
return (
<CascadingMenuButton
menuItems={[
{
label: 'Track order',
type: 'subMenu',
priority: 2000,
subMenu: [
{
label: minimized ? 'Restore track' : 'Minimize track',
icon: minimized ? AddIcon : MinimizeIcon,
onClick: () => {
track.setMinimized(!minimized)
},
},
...(view.tracks.length > 2
? [
{
label: 'Move track to top',
icon: KeyboardDoubleArrowUpIcon,
onClick: () => {
view.moveTrackToTop(track.id)
},
},
]
: []),

{
label: 'Move track up',
icon: KeyboardArrowUpIcon,
onClick: () => {
view.moveTrackUp(track.id)
},
},
{
label: 'Move track down',
icon: KeyboardArrowDownIcon,
onClick: () => {
view.moveTrackDown(track.id)
},
},
...(view.tracks.length > 2
? [
{
label: 'Move track to bottom',
icon: KeyboardDoubleArrowDownIcon,
onClick: () => {
view.moveTrackToBottom(track.id)
},
},
]
: []),
],
},
...(session.getTrackActionMenuItems?.(trackConf) || []),
...track.trackMenuItems(),
].sort((a, b) => (b?.priority || 0) - (a?.priority || 0))}
data-testid="track_menu_icon"
>
<MoreVertIcon fontSize="small" />
</CascadingMenuButton>
)
})

export default TrackLabel
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ exports[`renders one track, one region 1`] = `
class="MuiPaper-root MuiPaper-outlined MuiPaper-rounded css-8osoox-MuiPaper-root-root"
>
<div
class="MuiPaper-root MuiPaper-elevation MuiPaper-rounded MuiPaper-elevation1 css-9obt5s-MuiPaper-root-trackLabel-trackLabelOverlap-root"
class="MuiPaper-root MuiPaper-elevation MuiPaper-rounded MuiPaper-elevation1 css-166mc98-MuiPaper-root-trackLabel-trackLabelOverlap-root"
style="--Paper-shadow: 0px 2px 1px -1px rgba(0,0,0,0.2),0px 1px 1px 0px rgba(0,0,0,0.14),0px 1px 3px 0px rgba(0,0,0,0.12);"
>
<span
Expand Down Expand Up @@ -1215,7 +1215,7 @@ exports[`renders two tracks, two regions 1`] = `
class="MuiPaper-root MuiPaper-outlined MuiPaper-rounded css-8osoox-MuiPaper-root-root"
>
<div
class="MuiPaper-root MuiPaper-elevation MuiPaper-rounded MuiPaper-elevation1 css-9obt5s-MuiPaper-root-trackLabel-trackLabelOverlap-root"
class="MuiPaper-root MuiPaper-elevation MuiPaper-rounded MuiPaper-elevation1 css-166mc98-MuiPaper-root-trackLabel-trackLabelOverlap-root"
style="--Paper-shadow: 0px 2px 1px -1px rgba(0,0,0,0.2),0px 1px 1px 0px rgba(0,0,0,0.14),0px 1px 3px 0px rgba(0,0,0,0.12);"
>
<span
Expand Down Expand Up @@ -1338,7 +1338,7 @@ exports[`renders two tracks, two regions 1`] = `
class="MuiPaper-root MuiPaper-outlined MuiPaper-rounded css-8osoox-MuiPaper-root-root"
>
<div
class="MuiPaper-root MuiPaper-elevation MuiPaper-rounded MuiPaper-elevation1 css-9obt5s-MuiPaper-root-trackLabel-trackLabelOverlap-root"
class="MuiPaper-root MuiPaper-elevation MuiPaper-rounded MuiPaper-elevation1 css-166mc98-MuiPaper-root-trackLabel-trackLabelOverlap-root"
style="--Paper-shadow: 0px 2px 1px -1px rgba(0,0,0,0.2),0px 1px 1px 0px rgba(0,0,0,0.14),0px 1px 3px 0px rgba(0,0,0,0.12);"
>
<span
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,7 @@ exports[`<JBrowseLinearGenomeView /> renders successfully 1`] = `
class="MuiPaper-root MuiPaper-outlined MuiPaper-rounded css-8osoox-MuiPaper-root-root"
>
<div
class="MuiPaper-root MuiPaper-elevation MuiPaper-rounded MuiPaper-elevation1 css-15kbsoo-MuiPaper-root-trackLabel-trackLabelOffset-root"
class="MuiPaper-root MuiPaper-elevation MuiPaper-rounded MuiPaper-elevation1 css-1eqvr2m-MuiPaper-root-trackLabel-trackLabelOffset-root"
style="--Paper-shadow: 0px 2px 1px -1px rgba(0,0,0,0.2),0px 1px 1px 0px rgba(0,0,0,0.14),0px 1px 3px 0px rgba(0,0,0,0.12);"
>
<span
Expand Down

0 comments on commit 9d34f8c

Please sign in to comment.