ENG-1742 Add max height and scroll for node type menu

[trangdoan982]

https://www.loom.com/share/2c6b19da26f345a488119a7fe028e4d8

Summary

• Adds max-h-[60vh] overflow-y-auto to the node type menu so it doesn't overflow off-screen when there are many node types
• Scrolls the active item into view when navigating with arrow keys, so keyboard navigation works correctly in a scrollable menu

Fixes ENG-1742

Test plan

• Open the node menu (e.g. on a graph with many node types like discourse-graphs graph)
• Verify the menu is capped in height and scrollable
• Navigate with arrow keys and verify the active item scrolls into view
• Verify mouse selection and keyboard shortcut selection still work as expected

Made with Cursor

[linear-code]

ENG-1742

[supabase]

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[graphite-app]

The menuMaxHeight calculation can result in negative or extremely small values when there is insufficient vertical space below the menu position. This will cause the menu to be unusable or invisible.

Unlike the second calculation at lines 377-380 which uses Math.max(..., 100) to ensure a minimum height, this calculation has no safeguard.

Fix:

const menuMaxHeight = Math.max(
  window.innerHeight - (parentTop + coords.top) - 24,
  100
);

Suggested change

	const parentTop =
	props.textarea.parentElement?.getBoundingClientRect().top ?? 0;
	const menuMaxHeight = window.innerHeight - (parentTop + coords.top) - 24;
	const parentTop =
	props.textarea.parentElement?.getBoundingClientRect().top ?? 0;
	const menuMaxHeight = Math.max(
	window.innerHeight - (parentTop + coords.top) - 24,
	100
	);

Spotted by Graphite



Is this helpful? React 👍 or 👎 to let us know.

[trangdoan982]

i actually prefer without the max option, because that means all nodes will be visible within the view

if we set the max between the calculated value and 100, then user might still be unable to see the bottom of the list

[mdroidian]

@trangdoan982 https://www.loom.com/share/2b634b18a2884752826d4ca7501ea322

[trangdoan982]

Yes I enabled flipping from the Popover and here's the updated behavior
https://www.loom.com/share/1dfdf25d641f410b8f0d9f971d943820

[mdroidian]

Let's try to use the built in <Popover> position prop to deal with where is should render, instead of trying to render a very very small scrollable menu.

[mdroidian]

@trangdoan982 https://www.loom.com/share/2b634b18a2884752826d4ca7501ea322