feat: add context menu to file tabs with close actions #162
Conversation
|
|
||
| interface IContextMenuItems extends MenuProps { | ||
| key: ContextMenuKeys; | ||
| items: { |
There was a problem hiding this comment.
Nested objects should almost always be defined as a separate type alias. Necessity of IContextMenuItems['items'] supports this.
| @@ -22,6 +53,9 @@ const Tabs: FC = () => { | |||
| close(filePath); | |||
| }; | |||
|
|
|||
| const onMenuItemClick = (info: MenuInfo, filePath: Tree['path']) => { | |||
There was a problem hiding this comment.
This will force the rerender of the whole tab list every time Tabs component gets an update. Event handlers should be enclosed in useCallback.
src/hooks/fileTabs.hooks.ts
Outdated
| const updatedItems = fileTab.items.filter( | ||
| (item) => item.path === filePath, | ||
| ); | ||
| updatedTab = { |
There was a problem hiding this comment.
Mutable updatedTab unnecessarily increases code complexity.
It'd be much easier to return instead, and factor out syncTabSettings out of this function.
Also syncTabSettings gets exported along with open and close, even though it's semantically/architecturally a deeper concept. Probably it's worth it to check if there is a way to avoid returning syncTabSettings from this function, and incapsulate all the logic related to tab syncing here.
There was a problem hiding this comment.
I have removed syncTabSettings from this hook and made the required changes as suggested by you.
|
This is open for 3 weeks without progress. Should we close it? |
|
I was working on Misti support, so it was delayed. I'll resolve it soon. |
Resolves #72