Graphite icon indicating copy to clipboard operation
Graphite copied to clipboard

Refactor MenuList so its entries don't use the ref property

Open Keavon opened this issue 2 years ago • 0 comments

Currently this is a very janky, fragile, and incapable system that uses :ref="(ref: typeof FloatingMenu) => ref && (entry.ref = ref)" to append a ref attribute to child FloatingMenu entries by augmenting the data type:

export type MenuListEntry<Value = string> = MenuListEntryData<Value> & { ref?: typeof FloatingMenu | typeof MenuList; checked?: boolean };

This is a horrible hack with too many drawbacks. Augmenting the type also causes a lot of pain.

I think the best approach is to create a new state provider which globally handles sub-menus spawned by MenuList.

From a UX perspective, we also want the MenuBarInput to behave more nicely to be forgiving of user hover motion. A global handler would help make this possible. Perhaps we can even just entirely get rid of the idea of stray distance (which was inspired by Blender) from floating menus.

Fixing this will make it possible to properly handle checkboxes (like the File > Auto-Save checkbox) which is broken now. And we can fix the hacky way we hide a clicked menu list entry that relies on ref where we just set isOpen to false as a workaround.

Keavon avatar May 21 '22 12:05 Keavon