Skip to content

Conversation

@EnigmaXV
Copy link
Contributor

@EnigmaXV EnigmaXV commented Dec 8, 2025

  • Fixed issue where the kebab menu does not close when clicking outside the menu.
  • Added a useEffect hook to handle outside-click detection.
  • Ensured the menu behaves consistently with other Foreman UI dropdowns.

@github-actions github-actions bot added the UI label Dec 8, 2025
@EnigmaXV EnigmaXV force-pushed the 38936-fix-kebab-menu branch 3 times, most recently from 87085c8 to d6cfac7 Compare December 10, 2025 10:36
Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left one comment, overall looks good

const containerRef = React.useRef();
const containerRef = useRef();

useEffect(() => {
Copy link
Member

@chris1984 chris1984 Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, the event listeners are added and removed every time menuOpen toggles. However, if the menu is closed, you technically don't need to listen for clicks on the document at all.

You can optimize performance slightly by exiting the effect early if the menu is not open.

What do you think of this:

useEffect(() => {
  if (!menuOpen) return;

  const handleClickOutside = event => {
    if (containerRef.current && !containerRef.current.contains(event.target)) {
       setMenuOpen(false);
    }
  };

  document.addEventListener('mousedown', handleClickOutside);
  document.addEventListener('touchstart', handleClickOutside);

  return () => {
    document.removeEventListener('mousedown', handleClickOutside);
    document.removeEventListener('touchstart', handleClickOutside);
  };
}, [menuOpen, setMenuOpen]);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion. It makes total sense. I’ve updated the effect to early-return when menuOpen is false, so the document listeners are only attached while the menu is open. I also removed the extra menuOpen check inside the handler since it’s no longer needed.

Merry Christmas 🎄

@EnigmaXV EnigmaXV force-pushed the 38936-fix-kebab-menu branch from e557295 to fab8336 Compare December 31, 2025 13:40
@EnigmaXV EnigmaXV requested a review from chris1984 January 7, 2026 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants