feat: add auto-scroll to selected menu items in CustomSlashMenu

[jbronssin]

to resolve the issue #13047

It's my first contribution, I hope it's good enough!

When using the slash command (/) in the notes editor, a menu of commands appears. When navigating this menu with the up and down arrow keys, the selection changes, but the menu itself does not scroll. This means that as the list of commands is long, items outside the visible area cannot be seen when selected.

The issue was located in the [CustomSlashMenu.tsx] component. The menu container didn't have vertical scrolling enabled, and there was no logic to scroll the active item into the visible area.

The fix involved:

Adding overflow-y: auto to the menu's styled container in [CustomSlashMenu.tsx].
Modifying the [DropdownMenuItemsContainer] component to accept and forward a ref using React.forwardRef.
Implementing a useEffect hook in [CustomSlashMenu.tsx] that triggers on selection change. This hook uses a ref to the items container to call scrollIntoView({ block: 'nearest' }) on the currently selected menu item.

[greptile-apps]

PR Summary

Adds auto-scrolling functionality to slash command menu for better keyboard navigation in the editor.

• Modified CustomSlashMenu component with overflow-y: auto and implemented scrollIntoView for auto-scrolling selected items into view
• Refactored DropdownMenuItemsContainer to use forwardRef pattern enabling proper ref handling for scroll behavior
• Added container height management in StyledInnerContainer to ensure proper overflow handling
• Enhanced keyboard navigation accessibility by ensuring selected items remain visible during arrow key navigation
• Potential performance optimization needed for scrollIntoView behavior to avoid janky scrolling

_{2 files reviewed, 4 comments
Edit PR Review Bot Settings | Greptile}

[github-actions]

Welcome!

Hello there, congrats on your first PR! We're excited to have you contributing to this project.
By submitting your Pull Request, you acknowledge that you agree with the terms of our Contributor License Agreement.

Generated by 🚫 dangerJS against 4203650

[github-actions]

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:51103

This environment will automatically shut down when the PR is closed or after 5 hours.

[charlesBochet]

Hi @jbronssin, glad to see you here! We should catch up IRL

Regarding your PR:

• we avoid useEffects as much as possible (as this pattern introduce both race conditions and re-renders which is a nightmare do deal with as the codebase grows) ==> we will prefer synchronous ways. Here, when the event key down is pressed, and we move the selection, we can also make the scroll happen
• we avoid using forwardRef as this make the API more complex. Most of the time we don't need it

==> your implementation is not the right one according to our standards :)

You can take a look at the Filter dropdown for instance, you'll see that the auto-scrolling happens there. And you should see that the FilterDropdown is using ViewBarFilterDropdownContent itself using a SelectableList itself using a SelectableListItem

You'll see that SelectableListItem is doing exactly what I'm telling you not to do :p => useEffect. This is the oldway and we will likely refactor this component later.
For now, let's just use the SelectableList in the CommandSlashMenu and we should be all good!