#40539 - ContextMenu: scrollable and always visible by valeriu-vi · Pull Request #114 · flower-platform/react-timeline-10000

valeriu-vi · 2026-06-11T12:52:02Z

Proposed Change:

What is your change

Change type

Put an x in the boxes that apply

[ X] Bugfix
New feature

Checklist

I have added tests that prove my fix is effective or that my feature works
[ X] I have added necessary documentation (if appropriate)

gemini-code-assist

Code Review

This pull request enhances the ContextMenu component by tracking its coordinates in the state and adjusting its position to prevent viewport overflow. The review feedback recommends replacing the hardcoded global CONTEXT_MENU_ID with instance-specific unique IDs to avoid conflicts when multiple menus are rendered. Additionally, it suggests initializing the coordinates directly in the constructor to simplify the component's lifecycle methods and avoid redundant state updates.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-11T12:53:09Z

+const CONTEXT_MENU_ID = "CONTEXT_MENU_ID";
+
+export class ContextMenu extends React.Component<ContextMenuProps, { isOpened?: boolean, x: number, y: number }> {

  constructor(props) {
    super(props);
    this.close = this.close.bind(this);
    this.state = {
-      isOpened: props.positionToOpen ? true : false
+      isOpened: props.positionToOpen ? true : false, x: 0, y: 0
    }
  }


Using a hardcoded global CONTEXT_MENU_ID can cause conflicts and incorrect positioning calculations if multiple ContextMenu components are mounted on the page simultaneously.

Instead, we can generate a unique ID per instance using _.uniqueId and initialize the state coordinates directly in the constructor to avoid an unnecessary setState call on mount.

Suggested change

const CONTEXT_MENU_ID = "CONTEXT_MENU_ID";

export class ContextMenu extends React.Component<ContextMenuProps, { isOpened?: boolean, x: number, y: number }> {

constructor(props) {

super(props);

this.close = this.close.bind(this);

this.state = {

isOpened: props.positionToOpen ? true : false

isOpened: props.positionToOpen ? true : false, x: 0, y: 0

}

}

export class ContextMenu extends React.Component<ContextMenuProps, { isOpened?: boolean, x: number, y: number }> {

private menuId = _.uniqueId('context-menu-');

constructor(props) {

super(props);

this.close = this.close.bind(this);

this.state = {

isOpened: props.positionToOpen ? true : false,

x: props.positionToOpen?.x ?? 0,

y: props.positionToOpen?.y ?? 0

}

}

gemini-code-assist · 2026-06-11T12:53:09Z

+  adjustPopup(x: number, y: number) {
+    // Get the current dimensions of the popup element
+    const { width, height } = document.getElementById(CONTEXT_MENU_ID)?.getBoundingClientRect() || {} as DOMRect;
+
+    let finalX = x, finalY = y;
+    // If the popup overflows the right edge of the screen, flip it to the left side of the cursor
+    if (x + width > window.innerWidth) {
+      // Ensure it doesn't go off the left edge of the screen (min boundary of 0)
+      finalX = Math.max(0, x - width);
+    }
+
+    // If the popup overflows the bottom edge of the screen, flip it above the cursor
+    if (y + height > window.innerHeight) {
+      // Ensure it doesn't go off the top edge of the screen (min boundary of 0)
+      finalY = Math.max(0, y - height);
+    }
+
+    // Update the component state with the safe, adjusted coordinates
+    this.setState({ x: finalX, y: finalY })
+  }


Update adjustPopup to use the instance-specific this.menuId instead of the global CONTEXT_MENU_ID. Also, default width and height to 0 to prevent NaN calculations if the element is not found in the DOM.

Suggested change

adjustPopup(x: number, y: number) {

// Get the current dimensions of the popup element

const { width, height } = document.getElementById(CONTEXT_MENU_ID)?.getBoundingClientRect() || {} as DOMRect;

let finalX = x, finalY = y;

// If the popup overflows the right edge of the screen, flip it to the left side of the cursor

if (x + width > window.innerWidth) {

// Ensure it doesn't go off the left edge of the screen (min boundary of 0)

finalX = Math.max(0, x - width);

}

// If the popup overflows the bottom edge of the screen, flip it above the cursor

if (y + height > window.innerHeight) {

// Ensure it doesn't go off the top edge of the screen (min boundary of 0)

finalY = Math.max(0, y - height);

}

// Update the component state with the safe, adjusted coordinates

this.setState({ x: finalX, y: finalY })

}

adjustPopup(x: number, y: number) {

// Get the current dimensions of the popup element

const { width = 0, height = 0 } = document.getElementById(this.menuId)?.getBoundingClientRect() || {};

let finalX = x, finalY = y;

// If the popup overflows the right edge of the screen, flip it to the left side of the cursor

if (x + width > window.innerWidth) {

// Ensure it doesn't go off the left edge of the screen (min boundary of 0)

finalX = Math.max(0, x - width);

}

// If the popup overflows the bottom edge of the screen, flip it above the cursor

if (y + height > window.innerHeight) {

// Ensure it doesn't go off the top edge of the screen (min boundary of 0)

finalY = Math.max(0, y - height);

}

// Update the component state with the safe, adjusted coordinates

this.setState({ x: finalX, y: finalY })

}

gemini-code-assist · 2026-06-11T12:53:09Z

+      <Popup id={CONTEXT_MENU_ID} basic wide='very' data-testid={testids.popup} context={this.getPopupContext()}
+        style={{ maxHeight: '80vh', maxWidth: '80vh', overflow: 'auto' }}
+        positionFixed


Update the Popup component to use the instance-specific this.menuId instead of the global CONTEXT_MENU_ID.

Suggested change

<Popup id={CONTEXT_MENU_ID} basic wide='very' data-testid={testids.popup} context={this.getPopupContext()}

style={{ maxHeight: '80vh', maxWidth: '80vh', overflow: 'auto' }}

positionFixed

<Popup id={this.menuId} basic wide='very' data-testid={testids.popup} context={this.getPopupContext()}

style={{ maxHeight: '80vh', maxWidth: '80vh', overflow: 'auto' }}

positionFixed

gemini-code-assist · 2026-06-11T12:53:09Z

+  componentDidMount(): void {
+    this.props.positionToOpen && this.setState({ x: this.props.positionToOpen.x, y: this.props.positionToOpen.y }, () => {
+      this.props.positionToOpen && this.adjustPopup(this.props.positionToOpen.x, this.props.positionToOpen.y)
+    });
+  }


Since the initial coordinates are now set directly in the constructor, we can simplify componentDidMount to only call adjustPopup without the redundant setState call.

Suggested change

componentDidMount(): void {

this.props.positionToOpen && this.setState({ x: this.props.positionToOpen.x, y: this.props.positionToOpen.y }, () => {

this.props.positionToOpen && this.adjustPopup(this.props.positionToOpen.x, this.props.positionToOpen.y)

});

}

componentDidMount(): void {

if (this.props.positionToOpen) {

this.adjustPopup(this.props.positionToOpen.x, this.props.positionToOpen.y);

}

}

valeriu-vi · 2026-06-11T13:01:11Z

/gemini review

gemini-code-assist

Code Review

This pull request enhances the ContextMenu component to ensure it remains scrollable and fully visible within the viewport boundaries by dynamically adjusting its coordinates upon opening. The review feedback highlights several improvement opportunities: correcting a typo in the changelog, using the props parameter instead of this.props in the constructor to avoid initialization issues, resolving a potential flickering effect by introducing an isAdjusted state to defer visibility until coordinates are calculated, and fixing a typo in the CSS maxWidth property where 80vh was used instead of 80vw.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

valeriu-vi · 2026-06-11T13:09:34Z

/gemini review

gemini-code-assist

Code Review

This pull request makes the ContextMenu scrollable and ensures it remains visible within the viewport boundaries by dynamically adjusting its coordinates. The feedback highlights two critical issues with this implementation: first, shouldComponentUpdate is missing a check for nextState.isAdjusted, which can prevent the component from re-rendering and leave the menu permanently hidden; second, isAdjusted is not reset to false when positionToOpen changes, leading to a visible flicker as the menu briefly renders at unadjusted coordinates.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-11T13:10:45Z

+  shouldComponentUpdate(nextProps: ContextMenuProps, nextState: ContextMenuState) {
    return (
-      nextProps.positionToOpen !== this.props.positionToOpen ||
+      !_.isEqual(this.props.positionToOpen, nextProps.positionToOpen) ||
      !_.isEqual(nextProps.actions, this.props.actions) ||
      !_.isEqual(nextProps.paramsForAction, this.props.paramsForAction) ||
+      nextState.x !== this.state.x ||
+      nextState.y !== this.state.y ||
      nextState.isOpened !== this.state.isOpened
    );
  }


The shouldComponentUpdate method is missing a check for nextState.isAdjusted. If the context menu fits within the viewport boundaries without needing any coordinate adjustments (i.e., finalX === x and finalY === y), the state update isAdjusted: true will be triggered, but shouldComponentUpdate will return false because none of the other state properties changed. This will prevent the component from re-rendering, leaving the context menu permanently hidden (visibility: 'hidden').

Suggested change

shouldComponentUpdate(nextProps: ContextMenuProps, nextState: ContextMenuState) {

return (

nextProps.positionToOpen !== this.props.positionToOpen ||

!_.isEqual(this.props.positionToOpen, nextProps.positionToOpen) ||

!_.isEqual(nextProps.actions, this.props.actions) ||

!_.isEqual(nextProps.paramsForAction, this.props.paramsForAction) ||

nextState.x !== this.state.x ||

nextState.y !== this.state.y ||

nextState.isOpened !== this.state.isOpened

);

}

shouldComponentUpdate(nextProps: ContextMenuProps, nextState: ContextMenuState) {

return (

!_.isEqual(this.props.positionToOpen, nextProps.positionToOpen) ||

!_.isEqual(nextProps.actions, this.props.actions) ||

!_.isEqual(nextProps.paramsForAction, this.props.paramsForAction) ||

nextState.x !== this.state.x ||

nextState.y !== this.state.y ||

nextState.isAdjusted !== this.state.isAdjusted ||

nextState.isOpened !== this.state.isOpened

);

}

gemini-code-assist · 2026-06-11T13:10:45Z

  componentDidUpdate(prevProps: Readonly<ContextMenuProps>, prevState: Readonly<{}>, snapshot?: any): void {
-    if (this.props.positionToOpen != prevProps.positionToOpen) {
-      this.setState({ isOpened: this.props.positionToOpen ? true : false });
+    if (!_.isEqual(this.props.positionToOpen, prevProps.positionToOpen)) {
+      this.setState({ isOpened: this.props.positionToOpen ? true : false, x: this.props.positionToOpen?.x ?? 0, y: this.props.positionToOpen?.y ?? 0 }, () => {
+        this.props.positionToOpen && this.adjustPopup(this.props.positionToOpen.x, this.props.positionToOpen.y)
+      });
    }
  }


When positionToOpen changes, isAdjusted is not reset to false in the initial setState call. If the context menu was previously opened and adjusted, isAdjusted remains true. When it is opened again at a new position, it will briefly render as visible at the unadjusted coordinates before adjustPopup runs, causing a visible flicker. Resetting isAdjusted: false ensures it remains hidden until the new coordinates are calculated.

componentDidUpdate(prevProps: Readonly<ContextMenuProps>, prevState: Readonly<{}>, snapshot?: any): void { if (!_.isEqual(this.props.positionToOpen, prevProps.positionToOpen)) { this.setState({ isOpened: this.props.positionToOpen ? true : false, isAdjusted: false, x: this.props.positionToOpen?.x ?? 0, y: this.props.positionToOpen?.y ?? 0 }, () => { if (this.props.positionToOpen) { this.adjustPopup(this.props.positionToOpen.x, this.props.positionToOpen.y); } }); } }

valeriu-vi · 2026-06-11T13:15:06Z

/gemini review

gemini-code-assist

Code Review

This pull request updates the ContextMenu component to ensure it remains scrollable and visible within the viewport boundaries by dynamically adjusting its coordinates on mount and update. Feedback on these changes includes a critical issue where removing the position prop from the <Popup> component defaults it to 'top left', which breaks the coordinate adjustment math in adjustPopup. Additionally, it is recommended to update the type of prevState in componentDidUpdate to Readonly<ContextMenuState> for improved type safety.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-11T13:17:12Z

+      <Popup id={this.id} basic wide='very' data-testid={testids.popup} context={this.getPopupContext()}
+        style={{ maxHeight: '80vh', maxWidth: '80vw', overflow: 'auto', visibility: this.state.isAdjusted ? 'visible' : 'hidden' }}
+        positionFixed
        onClose={() => {
          this.close();
        }} open={(this.state.isOpened && visibleActions.length > 0)}>


The position prop was removed from the <Popup> component. In Semantic UI React, the default position for a Popup is 'top left', which renders the popup above the trigger element.

However, the coordinate adjustment logic in adjustPopup (lines 93-112) is designed under the assumption that the popup is rendered below the trigger (i.e., 'bottom left').

If the popup is rendered above the trigger by default:

If the cursor is near the top of the screen, the popup will overflow the top of the viewport, but adjustPopup will not detect or correct this because it only checks for bottom overflow (y + height > window.innerHeight).

If the cursor is near the bottom of the screen and bottom overflow is triggered, adjustPopup sets finalY = y - height. Since the popup is rendered above finalY, its top will be positioned at y - 2 * height, pushing it even further off-screen at the top.

To fix this and ensure the adjustment math works correctly, you should explicitly set the position prop to 'bottom left' (or preserve the custom position from paramsForAction with 'bottom left' as a fallback).

<Popup id={this.id} basic wide='very' data-testid={testids.popup} context={this.getPopupContext()} style={{ maxHeight: '80vh', maxWidth: '80vw', overflow: 'auto', visibility: this.state.isAdjusted ? 'visible' : 'hidden' }} positionFixed position={this.props.paramsForAction.position || 'bottom left'} onClose={() => { this.close(); }} open={(this.state.isOpened && visibleActions.length > 0)}>

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

valeriu-vi · 2026-06-11T13:21:57Z

/gemini review

gemini-code-assist

Code Review

This pull request updates the ContextMenu component to ensure it remains scrollable and visible within the viewport boundaries by dynamically adjusting its coordinates on mount and update. The feedback suggests replacing direct DOM querying (document.getElementById) with a React ref on the Menu component to measure dimensions in a more idiomatic and robust manner.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-11T13:23:20Z

+
+export class ContextMenu extends React.Component<ContextMenuProps, ContextMenuState> {
+
+  id = _.uniqueId("ContextMenu");


Using document.getElementById is an anti-pattern in React as it bypasses the virtual DOM and can lead to race conditions or bugs if multiple instances of the component exist. A more idiomatic and robust approach is to use a React ref to measure the popup's dimensions. Let's define a menuRef to reference the menu element.

Suggested change

id = _.uniqueId("ContextMenu");

menuRef = React.createRef<HTMLDivElement>();

NU merge asa cu ref ca intoarece un wraper semantic ui...

gemini-code-assist · 2026-06-11T13:23:21Z

+    // Get the current dimensions of the popup element
+    const { width = 0, height = 0 } = document.getElementById(this.id)?.getBoundingClientRect() || {} as DOMRect;


Update the adjustPopup method to measure the popup using the menuRef instead of querying the DOM by ID. This ensures we safely retrieve the dimensions from the React-managed DOM node.

Suggested change

// Get the current dimensions of the popup element

const { width = 0, height = 0 } = document.getElementById(this.id)?.getBoundingClientRect() || {} as DOMRect;

// Get the current dimensions of the popup element

const rect = this.menuRef.current?.getBoundingClientRect();

const width = rect?.width ?? 0;

const height = rect?.height ?? 0;

gemini-code-assist · 2026-06-11T13:23:21Z

+      <Popup id={this.id} basic wide='very' data-testid={testids.popup} context={this.getPopupContext()}
+        style={{ maxHeight: '80vh', maxWidth: '80vw', overflow: 'auto', visibility: this.state.isAdjusted ? 'visible' : 'hidden' }}
+        positionFixed
+        position='bottom left'
        onClose={() => {
          this.close();
        }} open={(this.state.isOpened && visibleActions.length > 0)}>
-        <Menu className="rct9k-context-menu" secondary vertical>
+        <Menu className="rct9k-context-menu" secondary vertical >


Remove the id prop from the Popup component and attach the menuRef directly to the Menu component. This allows us to measure the menu's dimensions directly and cleanly.

Suggested change

<Popup id={this.id} basic wide='very' data-testid={testids.popup} context={this.getPopupContext()}

style={{ maxHeight: '80vh', maxWidth: '80vw', overflow: 'auto', visibility: this.state.isAdjusted ? 'visible' : 'hidden' }}

positionFixed

position='bottom left'

onClose={() => {

this.close();

}} open={(this.state.isOpened && visibleActions.length > 0)}>

<Menu className="rct9k-context-menu" secondary vertical>

<Menu className="rct9k-context-menu" secondary vertical >

<Popup basic wide='very' data-testid={testids.popup} context={this.getPopupContext()}

style={{ maxHeight: '80vh', maxWidth: '80vw', overflow: 'auto', visibility: this.state.isAdjusted ? 'visible' : 'hidden' }}

positionFixed

position='bottom left'

onClose={() => {

this.close();

}} open={(this.state.isOpened && visibleActions.length > 0)}>

<Menu ref={this.menuRef} className="rct9k-context-menu" secondary vertical >

#40539 - ContextMenu: scrolable and visible

4d08cff

gemini-code-assist Bot reviewed Jun 11, 2026

View reviewed changes

valeriu-vi added 2 commits June 11, 2026 15:53

#40539 - update CHANGELOG.md

0368137

#40539 - solve AI remarks

d39aa17

gemini-code-assist Bot reviewed Jun 11, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Comment thread src/components/ContextMenu/ContextMenu.tsx Outdated

Comment thread src/components/ContextMenu/ContextMenu.tsx

Comment thread src/components/ContextMenu/ContextMenu.tsx Outdated

valeriu-vi and others added 5 commits June 11, 2026 16:03

Update CHANGELOG.md

e3549a7

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Update src/components/ContextMenu/ContextMenu.tsx

1d2f7ed

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Update src/components/ContextMenu/ContextMenu.tsx

8647186

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Update src/components/ContextMenu/ContextMenu.tsx

56a2d15

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

#40539 - solve AI remarks

7ae21c1

gemini-code-assist Bot reviewed Jun 11, 2026

View reviewed changes

valeriu-vi added 2 commits June 11, 2026 16:14

#40539 - solve AI remarks

4c63822

#40539 - solve AI remarks

646a219

gemini-code-assist Bot reviewed Jun 11, 2026

View reviewed changes

valeriu-vi and others added 2 commits June 11, 2026 16:19

#40539 - solve AI remarks

7d5663f

Update src/components/ContextMenu/ContextMenu.tsx

cdcfb13

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

gemini-code-assist Bot reviewed Jun 11, 2026

View reviewed changes

valeriu-vi changed the title ~~#40539 - ContextMenu: scrolable and visible~~ #40539 - ContextMenu: scrollable and always visible Jun 11, 2026

valeriu-vi added 6 commits June 11, 2026 16:29

#40539 - update CHANGELOG.md

28c5d63

#40539 - fix tests

fdf379a

#40539 - fix tests

ba5d053

#40539 - fix build

d64ea27

#40539 - fix build

273949d

#40539 - remove console.log

f8885bb

cristian-spiescu merged commit a772532 into master-flower-platform Jun 27, 2026
2 checks passed

cristian-spiescu deleted the valeriu-RM40539-ContextMenu-scrollable-visible branch June 27, 2026 08:32


		export class ContextMenu extends React.Component<ContextMenuProps, ContextMenuState> {

		id = _.uniqueId("ContextMenu");

	id = _.uniqueId("ContextMenu");
	menuRef = React.createRef<HTMLDivElement>();

		// Get the current dimensions of the popup element
		const { width = 0, height = 0 } = document.getElementById(this.id)?.getBoundingClientRect() \|\| {} as DOMRect;

Uh oh!

Conversation

valeriu-vi commented Jun 11, 2026

Proposed Change:

Change type

Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

valeriu-vi commented Jun 11, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

valeriu-vi commented Jun 11, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

valeriu-vi commented Jun 11, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

valeriu-vi commented Jun 11, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!