lexical icon indicating copy to clipboard operation
lexical copied to clipboard

Bug: Typeahead menu rendering off-screen when cursor near edges

Open aalmazan opened this issue 2 years ago • 10 comments

Lexical version: latest master @ f6a278eac8c8ac1c3f1523df5591b57e89c6cda8

Steps To Reproduce

  1. Open Lexical playground.
  2. Move cursor over to the edge of the editor area. Cursor should be close to the right-edge of the browser window -- at least this is how it's demonstrated in the linked repro video.
  3. Use @-mention feature to trigger the typeahead menu from LexicalTypeaheadMenuPlugin.
  4. Menu will render outside of visible area.

Note: This is easier to reproduce if the browser is resized to a smaller width and/or the browser's responsive design mode is enabled.

Link to code example:

https://user-images.githubusercontent.com/800948/217070201-8857417a-9cb6-4bc6-b193-78fbad4c44e5.mp4

The current behavior

Currently the typeahead menu from LexicalTypeaheadMenuPlugin renders outside of the user's viewable area when triggered near the edge of the browser window. The cursor position where this happens can depend based on the width of the content that's inside of the menu. This is likely also an issue if the cursor position is near the bottom and bottom right of the window.

The expected behavior

The typeahead menu is expected to possibly have a maximum location based on the window's height and width and/or maybe run developer-defined function to change the positioning depending on the situation's needs.

aalmazan avatar Feb 06 '23 19:02 aalmazan

I face the same issue. I see packages/lexical-react/src/LexicalTypeaheadMenuPlugin.tsx has a useDynamicPositioning hook but it doesn't work.

svk-assembly avatar Feb 07 '23 10:02 svk-assembly

Does #3826 fix the issue for either of you?

thegreatercurve avatar Feb 07 '23 14:02 thegreatercurve

Unfortunately no that doesn't work -- although I think it resolves another issue I've seen that was similar.

@svk-assembly and for those that are looking for a workaround for now: You can provide a different element ref for LexicalTypeaheadMenuPlugin's menuRenderFn prop then maybe use some CSS trickery to position it a little better. Doing this, however, makes the menu element "unaware" of the current cursor position.

https://github.com/facebook/lexical/blob/14cd2a6ce853996c1f3afe8b3aa14b52f29c5ba4/packages/lexical-playground/src/plugins/MentionsPlugin/index.tsx#L684-L720

In the playground example, you would change line 715 to an element ref you provide (maybe from a parent component) instead.

aalmazan avatar Feb 07 '23 17:02 aalmazan

if this issue is happening because of https://github.com/facebook/lexical/pull/3826, could I try to fix it?

blacktoast avatar Feb 09 '23 16:02 blacktoast

I'm dealing with the same issue. The overflow is not only happening on the right side but also at the bottom if Lexical is placed far at the bottom of the page.

In addition to @aalmazan findings, I would like to add that there is an anchorClassName prop on the LexicalTypeaheadMenuPlugin component that passes all classes to the anchor element ref. So it's possible to apply styles to the anchor element while keeping the automatic position feature with anchorElementRef.

However, that doesn't solve the problem. I was thinking about adding some collision detection myself and "mirror the direction" of the typeahead (right -> left and bottom -> top) if there is any collision detected. In my use case, I'm fine with screen collision detection. I imagine others may prefer to have a collision detection with the Lexical instance so the typeahead component would never overflow the editor.

davidwlhlm avatar Feb 09 '23 22:02 davidwlhlm

I wrote this plugin, and unfortunately there is a fine line between building a decent typeahead plugin for a text editor and building an entire popover framework with robust positioning heuristics (which there are already plenty of).

I think a good compromise would be allowing the consumer of this plugin to have more control of the positioning, thoughts? Or within menuRenderFn you can render a component that handles all of it's own positioning, that's what we do internally.

tylerjbainbridge avatar Mar 01 '23 16:03 tylerjbainbridge

Until this is fixed, there is an alternative solution using @floating-ui lib combined with this plugin. Here's the sample implementation:

import {
  useFloating,
  FloatingPortal,
  autoPlacement,
  offset,
  flip,
  shift,
} from '@floating-ui/react';

function ComponentPickerMenuPlugin({ anchorEl, options}) {
   const { x, y, positionReference, refs, strategy } = useFloating({
    placement: 'top',
    middleware: [offset(15), autoPlacement(), shift()],
  });
 ///..... other function as seen on playground.

   <LexicalTypeaheadMenuPlugin<ComponentPickerOption>
      onQueryChange={setQueryString}
      onSelectOption={onSelectOption}
      triggerFn={checkForTriggerMatch}
      onOpen={(r) => {
        positionReference({
          getBoundingClientRect: r.getRect
        });
      }}
      options={filteredOptions}
      anchorClassName="reset-anchor"
      menuRenderFn={(
        anchorElementRef,
        { selectedIndex, selectOptionAndCleanUp, setHighlightedIndex }
      ) =>
        anchorElementRef.current && filteredOptions.length && isMenuOpen ? (
          <FloatingPortal root={anchorEl}>
            <div
              ref={refs.setFloating}
              style={{
                position: strategy,
                top: y ?? 0,
                left: x ?? 0,
                width: 'max-content',
              }}>
              
                <ul>
                  {filteredOptions.map((option, i: number) => (
                    <ComponentPickerMenuItem
                      index={i}
                      isSelected={selectedIndex === i}
                      onClick={() => {
                        setHighlightedIndex(i);
                        selectOptionAndCleanUp(option);
                      }}
                      onMouseEnter={() => {
                        setHighlightedIndex(i);
                      }}
                      key={option.key}
                      option={option}
                    />
                  ))}
                </ul>
            </div>
          </FloatingPortal>
        ) : null
      }
    />
}

.reset-anchor {
 top: initial!important;
 left: initial!important;
}

eashish93 avatar Mar 21 '23 09:03 eashish93

@tylerjbainbridge I completely agree this should be more composable with other libraries whose purposes are positioning popovers like floating-ui and ariakit

bitofbreeze avatar Jun 19 '23 02:06 bitofbreeze

Thanks @eashish93. I've got a little improved version below. positionReference is not available according to my types and latest version of floating-ui. Instead, use refs.setPositionReference.

Below is my version. I'm using flip() (docs) which is a "no-space" fallback strategy, meaning, it will only re-position when there's no space.

const { x, y, refs, strategy } = useFloating({
  placement: 'bottom-start',
  middleware: [
    offset(6),
    flip(),
    shift()
  ],
});
onOpen={(r) => {
  refs.setPositionReference({
    getBoundingClientRect: r.getRect,
  });
}}

jvandenaardweg avatar Jun 19 '23 05:06 jvandenaardweg

Another annoying bad UX is that the menu changes its positioning - above or under the caret - depending on the number of items.

Steps to reproduce:

  1. open the playground
  2. delete everything there
  3. press enter 5 times
  4. type / - the menu appears under the caret
  5. type pa - the menu changes its position
  6. type r - the menu changes its position again

From a user's perspective, this jumping is distracting and doesn't make sense. IMHO the menu shouldn't take into account the boundaries of the editor element, only the window's boundaries.

thorn0 avatar Sep 18 '23 22:09 thorn0