actualbudget / actual

A local-first personal finance app
https://actualbudget.org
MIT License
14.34k stars 1.15k forks source link

[Bug]: Can't click off to close filter menu with select open #3051

Open matt-fidd opened 3 months ago

matt-fidd commented 3 months ago

Verified issue does not already exist?

What happened?

Video below for reproduction. If you open a filter modal (popover?) with a select box open (eg category one of), you can no longer click off to close the whole thing. You must first click off the select, then you can click off the filter.

https://github.com/user-attachments/assets/4d07c034-3219-4858-b41a-0d34ec7e672c

Where are you hosting Actual?

Locally via Yarn

What browsers are you seeing the problem on?

Chrome

Operating System

Windows 11

psybers commented 3 months ago

Probably related: #3039

lelemm commented 16 hours ago

As far as I can tell, the problem here is this underlay invalidating the click action:

image

Looking at the components involved, was not able to find which component is creating this underlay. Tried to check Downshift source code and didn't find it.

If you hack the CSS like this it works:

image

Couldn't find any drawback of not having this underlay

psybers commented 16 hours ago

Interesting. The only things that add "underlay" are from react-aria as part of their popover.

psybers commented 16 hours ago
image
lelemm commented 13 hours ago

react-aria Popover.tsx:

  return (
    <Overlay isExiting={isExiting} portalContainer={UNSTABLE_portalContainer}>
      {!props.isNonModal && state.isOpen && <div data-testid="underlay" {...underlayProps} style={{position: 'fixed', inset: 0}} />}
      <div
        {...mergeProps(filterDOMProps(props as any), popoverProps)}
        {...renderProps}
        ref={ref}
        slot={props.slot || undefined}
        style={style}
        data-trigger={props.trigger}
        data-placement={placement}
        data-entering={isEntering || undefined}
        data-exiting={isExiting || undefined}>
        {!props.isNonModal && <DismissButton onDismiss={state.close} />}
        <OverlayArrowContext.Provider value={{...arrowProps, placement, ref: arrowRef}}>
          {renderProps.children}
        </OverlayArrowContext.Provider>
        <DismissButton onDismiss={state.close} />
      </div>
    </Overlay>

This is the aria source code of the underlay for the popover.

Tried to put "isNonModal=true" to remove the underlay, it removes as expected BUT it introduces another problem. If only works when you click at some places:

https://github.com/user-attachments/assets/0dec23a2-a3cc-4ea0-83c9-7c750ee3199f

Reading more of the react aria code, I found this: source: useOverlay.ts#L147:

  let onPointerDownUnderlay = e => {
    // fixes a firefox issue that starts text selection https://bugzilla.mozilla.org/show_bug.cgi?id=1675846
    if (e.target === e.currentTarget) {
      e.preventDefault();
    }
  };

This is what makes the onOpenChange from the Popover never get called when underlay is present.

The only way I was able to consistently close the popover is to adding

        shouldCloseOnInteractOutside={element => {
          // Datepicker selections for some reason register 2x clicks
          // We want to keep the popover open after selecting a date.
          // So we ignore the "close" event on selection + the subsequent event.
          if (element.dataset.pikaYear) {
            isDatepickerClick = true;
            return false;
          }
          if (isDatepickerClick) {
            isDatepickerClick = false;
            return false;
          }

+         dispatch({ type: 'close' });
          return true;
        }}

here FiltersMenu.jsx This looks like a hack tbh