Fixed dropdown menu being cut off in comments-ui

closes https://linear.app/ghost/issue/PLG-273

- removed previous fix which only worked on last comment but not last reply
  - keeping track of last comment/reply spread a lot of domain knowledge around for a UI-only concern and wouldn't scale if we have other dropdowns in the future
- added `useOutOfViewportClasses` hook
  - accepts an object with top/bottom/right/left containing default and outOfViewPort classes
  - applies the correct classes using the DOM rather than React so that we avoid re-renders and associated flickering or broken rendering

apps/comments-ui/src/components/content/buttons/MoreButton.tsx

@@ -10,7 +10,7 @@ type Props = {
 
 const MoreButton: React.FC<Props> = ({comment, toggleEdit}) => {
     const [isContextMenuOpen, setIsContextMenuOpen] = useState(false);
-    const {member, admin, pagination, comments} = useAppContext();
+    const {member, admin} = useAppContext();
     const isAdmin = !!admin;
 
     const toggleContextMenu = () => {
@@ -21,10 +21,6 @@ const MoreButton: React.FC<Props> = ({comment, toggleEdit}) => {
         setIsContextMenuOpen(false);
     };
 
-    // Check if this is the last comment and there's no more pagination
-    const isLastComment = (!pagination || pagination.total <= pagination.page * pagination.limit) && 
-                         comments[comments.length - 1]?.id === comment.id;
-
     const show = (!!member && comment.status === 'published') || isAdmin;
 
     if (!show) {
@@ -36,7 +32,7 @@ const MoreButton: React.FC<Props> = ({comment, toggleEdit}) => {
             <button className="outline-0" type="button" onClick={toggleContextMenu}>
                 <MoreIcon className={`duration-50 gh-comments-icon gh-comments-icon-more outline-0 transition ease-linear hover:fill-black/75 dark:hover:fill-white/75 ${isContextMenuOpen ? 'fill-black/75 dark:fill-white/75' : 'fill-black/50 dark:fill-white/60'}`} />
             </button>
-            {isContextMenuOpen ? <CommentContextMenu close={closeContextMenu} comment={comment} isLastComment={isLastComment} toggleEdit={toggleEdit} /> : null}
+            {isContextMenuOpen ? <CommentContextMenu close={closeContextMenu} comment={comment} toggleEdit={toggleEdit} /> : null}
         </div>
     );
 };

apps/comments-ui/src/components/content/context-menus/CommentContextMenu.test.jsx

@@ -0,0 +1,40 @@
+import CommentContextMenu from './CommentContextMenu';
+import React from 'react';
+import sinon from 'sinon';
+import {AppContext} from '../../../AppContext';
+import {buildComment} from '../../../../test/utils/fixtures';
+import {render, screen} from '@testing-library/react';
+
+const contextualRender = (ui, {appContext, ...renderOptions}) => {
+    const contextWithDefaults = {
+        member: null,
+        dispatchAction: () => {},
+        t: str => str,
+        ...appContext
+    };
+
+    return render(
+        <AppContext.Provider value={contextWithDefaults}>{ui}</AppContext.Provider>,
+        renderOptions
+    );
+};
+
+describe('<CommentContextMenu>', () => {
+    afterEach(() => {
+        sinon.restore();
+    });
+
+    it('has display-below classes when in viewport', () => {
+        const comment = buildComment();
+        contextualRender(<CommentContextMenu comment={comment} />, {appContext: {admin: true, labs: {commentImprovements: true}}});
+        expect(screen.getByTestId('comment-context-menu-inner')).toHaveClass('top-0');
+    });
+
+    it('has display-above classes when bottom is out of viewport', () => {
+        sinon.stub(HTMLElement.prototype, 'getBoundingClientRect').returns({bottom: 2000});
+
+        const comment = buildComment();
+        contextualRender(<CommentContextMenu comment={comment} />, {appContext: {admin: true, labs: {commentImprovements: true}}});
+        expect(screen.getByTestId('comment-context-menu-inner')).toHaveClass('bottom-full', 'mb-6');
+    });
+});

apps/comments-ui/src/components/content/context-menus/CommentContextMenu.tsx

@@ -3,20 +3,30 @@ import AuthorContextMenu from './AuthorContextMenu';
 import NotAuthorContextMenu from './NotAuthorContextMenu';
 import {Comment, useAppContext, useLabs} from '../../../AppContext';
 import {useEffect, useRef} from 'react';
+import {useOutOfViewportClasses} from '../../../utils/hooks';
 
 type Props = {
     comment: Comment;
     close: () => void;
     toggleEdit: () => void;
-    isLastComment?: boolean;
 };
-const CommentContextMenu: React.FC<Props> = ({comment, close, toggleEdit, isLastComment}) => {
+const CommentContextMenu: React.FC<Props> = ({comment, close, toggleEdit}) => {
     const {member, admin} = useAppContext();
     const isAuthor = member && comment.member?.uuid === member?.uuid;
     const isAdmin = !!admin;
     const element = useRef<HTMLDivElement>(null);
+    const innerElement = useRef<HTMLDivElement>(null);
     const labs = useLabs();
-    
+
+    // By default display dropdown below but move above if that renders off-screen
+    // NOTE: innerElement ref is only set when commentImprovements flag is enabled
+    useOutOfViewportClasses(innerElement, {
+        bottom: {
+            default: 'top-0',
+            outOfViewport: 'bottom-full mb-6'
+        }
+    });
+
     useEffect(() => {
         const listener = () => {
             close();
@@ -79,8 +89,8 @@ const CommentContextMenu: React.FC<Props> = ({comment, close, toggleEdit, isLast
 
     return (
         labs.commentImprovements ? (
-            <div ref={element} className="relative" onClick={stopPropagation}>
-                <div className={`absolute z-10 min-w-min whitespace-nowrap rounded bg-white p-1 font-sans text-sm shadow-lg outline-0 sm:min-w-[80px] dark:bg-neutral-800 dark:text-white ${isLastComment ? 'bottom-full mb-6' : 'top-0'}`}>
+            <div ref={element} className="relative" data-testid="comment-context-menu" onClick={stopPropagation}>
+                <div ref={innerElement} className={`absolute z-10 min-w-min whitespace-nowrap rounded bg-white p-1 font-sans text-sm shadow-lg outline-0 sm:min-w-[80px] dark:bg-neutral-800 dark:text-white`} data-testid="comment-context-menu-inner">
                     {contextMenu}
                 </div>
             </div>

apps/comments-ui/src/components/content/context-menus/NotAuthorContextMenu.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import {useAppContext} from '../../../AppContext';
+import {Comment, useAppContext} from '../../../AppContext';
 
 type Props = {
     comment: Comment;

apps/comments-ui/src/utils/hooks.test.tsx

@@ -0,0 +1,74 @@
+import React from 'react';
+import sinon from 'sinon';
+import {fireEvent, render, screen} from '@testing-library/react';
+import {useOutOfViewportClasses} from './hooks';
+
+describe('useOutOfViewportClasses', () => {
+    const classes = {
+        top: {default: 'default-top', outOfViewport: 'out-top'},
+        bottom: {default: 'default-bottom', outOfViewport: 'out-bottom'},
+        left: {default: 'default-left', outOfViewport: 'out-left'},
+        right: {default: 'default-right', outOfViewport: 'out-right'}
+    };
+
+    const TestComponent = () => {
+        const ref = React.useRef<HTMLDivElement>(null);
+        useOutOfViewportClasses(ref, classes);
+
+        // eslint-disable-next-line i18next/no-literal-string
+        return <div ref={ref} data-testid="test-element">Test element</div>;
+    };
+
+    afterEach(() => {
+        sinon.restore();
+    });
+
+    it('should apply default classes on mount when in viewport', () => {
+        render(<TestComponent />);
+
+        const element = screen.getByTestId('test-element');
+        expect(element).toHaveClass('default-top', 'default-bottom', 'default-left', 'default-right');
+    });
+
+    it('should apply outOfViewport classes on mount when out of viewport', () => {
+        sinon.stub(HTMLElement.prototype, 'getBoundingClientRect').returns({
+            top: -100, // out of viewport
+            bottom: 2000, // out of viewport (jest-dom default height: 768)
+            left: -5, // out of viewport
+            right: 2000, // out of viewport (jest-dom default width: 1024)
+            width: 100,
+            height: 50,
+            x: 0,
+            y: 0,
+            toJSON: () => ({})
+        });
+
+        render(<TestComponent />);
+
+        const element = screen.getByTestId('test-element');
+        expect(element).toHaveClass('out-top', 'out-bottom', 'out-left', 'out-right');
+    });
+
+    it('should apply outOfViewport classes when element moves out of viewport on resize', () => {
+        render(<TestComponent />);
+
+        const element = screen.getByTestId('test-element');
+        expect(element).toHaveClass('default-top', 'default-bottom', 'default-left', 'default-right');
+
+        sinon.stub(HTMLElement.prototype, 'getBoundingClientRect').returns({
+            top: -100, // out of viewport
+            bottom: 2000, // out of viewport (jest-dom default height: 768)
+            left: -5, // out of viewport
+            right: 2000, // out of viewport (jest-dom default width: 1024)
+            width: 100,
+            height: 50,
+            x: 0,
+            y: 0,
+            toJSON: () => ({})
+        });
+
+        fireEvent.resize(window);
+
+        expect(element).toHaveClass('out-top', 'out-bottom', 'out-left', 'out-right');
+    });
+});

apps/comments-ui/src/utils/hooks.ts

@@ -2,7 +2,7 @@ import {CommentsEditorConfig, getEditorConfig} from './editor';
 import {Editor, useEditor as useTiptapEditor} from '@tiptap/react';
 import {formatRelativeTime} from './helpers';
 import {useAppContext} from '../AppContext';
-import {useCallback, useEffect, useMemo, useRef, useState} from 'react';
+import {useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState} from 'react';
 
 /**
  * Execute a callback when a ref is set and unset.
@@ -76,3 +76,81 @@ export function useEditor(editorConfig: CommentsEditorConfig, initialHasContent
         hasContent
     };
 }
+
+type OutOfViewport = {
+    top: boolean;
+    bottom: boolean;
+    left: boolean;
+    right: boolean;
+}
+type OutOfViewportClassOptions = {
+    default: string;
+    outOfViewport: string;
+}
+type OutOfViewportClasses = {
+    top?: OutOfViewportClassOptions;
+    bottom?: OutOfViewportClassOptions;
+    left?: OutOfViewportClassOptions;
+    right?: OutOfViewportClassOptions;
+};
+// TODO: This does not currently handle the case where the element is outOfViewport for both top&bottom or left&right
+export function useOutOfViewportClasses(ref: React.RefObject<HTMLElement>, classes: OutOfViewportClasses) {
+    // Add/Remove classes directly on the element based on whether it's out of the viewport
+    // Modifies element classes directly in DOM so it's compatible with useLayoutEffect
+    const applyDefaultClasses = useCallback(() => {
+        if (ref.current) {
+            for (const value of Object.values(classes)) {
+                ref.current.classList.add(...value.default.split(' '));
+                ref.current.classList.remove(...value.outOfViewport.split(' '));
+            }
+        }
+    }, [ref, classes]);
+
+    const applyOutOfViewportClasses = useCallback((outOfViewport: OutOfViewport) => {
+        if (ref.current) {
+            for (const [side, sideClasses] of Object.entries(classes)) {
+                if (outOfViewport[side as keyof OutOfViewport]) {
+                    ref.current.classList.add(...sideClasses.outOfViewport.split(' '));
+                    ref.current.classList.remove(...sideClasses.default.split(' '));
+                } else {
+                    ref.current.classList.add(...sideClasses.default.split(' '));
+                    ref.current.classList.remove(...sideClasses.outOfViewport.split(' '));
+                }
+            }
+        }
+    }, [ref, classes]);
+
+    const updateOutOfViewportClasses = useCallback(() => {
+        if (ref.current) {
+            // Handle element being inside an iframe
+            const _document = ref.current.ownerDocument;
+            const _window = _document.defaultView || window;
+
+            // Reset classes so we can re-calculate without any previous re-positioning affecting the calcs
+            applyDefaultClasses();
+
+            const bounding = ref.current.getBoundingClientRect();
+            const outOfViewport = {
+                top: bounding.top < 0,
+                bottom: bounding.bottom > (_window.innerHeight || _document.documentElement.clientHeight),
+                left: bounding.left < 0,
+                right: bounding.right > (_window.innerWidth || _document.documentElement.clientWidth)
+            };
+
+            applyOutOfViewportClasses(outOfViewport);
+        }
+    }, [ref]);
+
+    // Layout effect needed here to avoid flicker of the default position before
+    // repositioning the element
+    useLayoutEffect(() => {
+        updateOutOfViewportClasses();
+    }, [ref]);
+
+    useEffect(() => {
+        window.addEventListener('resize', updateOutOfViewportClasses);
+        return () => {
+            window.removeEventListener('resize', updateOutOfViewportClasses);
+        };
+    }, []);
+}