ScreenTabs refactor (#182)

* inital implementation

* cleanup

* ui jump fix

* fix wrong first tab top right border radius

* add comment

* remove react.fragment.  only do initial screens populate on componentDidMount, rename this.scrollIntoViewTimeout (add Id to distinguish from state.scrollIntoViewTimeout).

* clean up some imports

* use single tabRef

src/app/workspace/screen/tab.tsx

@@ -0,0 +1,131 @@
+// Copyright 2023, Command Line Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+import * as React from "react";
+import * as mobxReact from "mobx-react";
+import * as mobx from "mobx";
+import { sprintf } from "sprintf-js";
+import { boundMethod } from "autobind-decorator";
+import { For } from "tsx-control-statements/components";
+import cn from "classnames";
+import { GlobalModel, GlobalCommandRunner, Session, Screen } from "../../../model/model";
+import { renderCmdText } from "../../common/common";
+import { ReactComponent as SquareIcon } from "../../assets/icons/tab/square.svg";
+import { ReactComponent as ActionsIcon } from "../../assets/icons/tab/actions.svg";
+import * as constants from "../../appconst";
+import { Reorder } from "framer-motion";
+import { MagicLayout } from "../../magiclayout";
+
+@mobxReact.observer
+class ScreenTab extends React.Component<
+    { screen: Screen; activeScreenId: string; index: number; onSwitchScreen: (screenId: string) => void },
+    {}
+> {
+    tabRef = React.createRef<HTMLUListElement>();
+    dragEndTimeout = null;
+    scrollIntoViewTimeout = null;
+
+    componentWillUnmount() {
+        if (this.scrollIntoViewTimeout) {
+            clearTimeout(this.dragEndTimeout);
+        }
+    }
+
+    @boundMethod
+    handleDragEnd() {
+        if (this.dragEndTimeout) {
+            clearTimeout(this.dragEndTimeout);
+        }
+
+        // Wait for the animation to complete
+        this.dragEndTimeout = setTimeout(() => {
+            const tabElement = this.tabRef.current;
+            if (tabElement) {
+                const finalTabPosition = tabElement.offsetLeft;
+
+                // Calculate the new index based on the final position
+                const newIndex = Math.floor(finalTabPosition / MagicLayout.TabWidth);
+
+                GlobalCommandRunner.screenReorder(this.props.screen.screenId, `${newIndex + 1}`);
+            }
+        }, 100);
+    }
+
+    @boundMethod
+    openScreenSettings(e: any, screen: Screen): void {
+        e.preventDefault();
+        e.stopPropagation();
+        mobx.action(() => {
+            GlobalModel.screenSettingsModal.set({ sessionId: screen.sessionId, screenId: screen.screenId });
+        })();
+        GlobalModel.modalsModel.pushModal(constants.SCREEN_SETTINGS);
+    }
+
+    renderTabIcon = (screen: Screen): React.ReactNode => {
+        const tabIcon = screen.getTabIcon();
+        if (tabIcon === "default" || tabIcon === "square") {
+            return (
+                <div className="icon svg-icon">
+                    <SquareIcon className="left-icon" />
+                </div>
+            );
+        }
+        return (
+            <div className="icon fa-icon">
+                <i className={`fa-sharp fa-solid fa-${tabIcon}`}></i>
+            </div>
+        );
+    };
+
+    render() {
+        let { screen, activeScreenId, index, onSwitchScreen } = this.props;
+
+        let tabIndex = null;
+        if (index + 1 <= 9) {
+            tabIndex = <div className="tab-index">{renderCmdText(String(index + 1))}</div>;
+        }
+        let settings = (
+            <div onClick={(e) => this.openScreenSettings(e, screen)} title="Actions" className="tab-gear">
+                <ActionsIcon className="icon hoverEffect " />
+            </div>
+        );
+        let archived = screen.archived.get() ? (
+            <i title="archived" className="fa-sharp fa-solid fa-box-archive" />
+        ) : null;
+
+        let webShared = screen.isWebShared() ? (
+            <i title="shared to web" className="fa-sharp fa-solid fa-share-nodes web-share-icon" />
+        ) : null;
+
+        return (
+            <Reorder.Item
+                ref={this.tabRef}
+                value={screen}
+                id={"screentab-" + screen.screenId}
+                whileDrag={{
+                    backgroundColor: "rgba(13, 13, 13, 0.85)",
+                }}
+                data-screenid={screen.screenId}
+                className={cn(
+                    "screen-tab",
+                    { "is-active": activeScreenId == screen.screenId, "is-archived": screen.archived.get() },
+                    "color-" + screen.getTabColor()
+                )}
+                onPointerDown={() => onSwitchScreen(screen.screenId)}
+                onContextMenu={(event) => this.openScreenSettings(event, screen)}
+                onDragEnd={this.handleDragEnd}
+            >
+                {this.renderTabIcon(screen)}
+                <div className="tab-name truncate">
+                    {archived}
+                    {webShared}
+                    {screen.name.get()}
+                </div>
+                {tabIndex}
+                {settings}
+            </Reorder.Item>
+        );
+    }
+}
+
+export { ScreenTab };

src/app/workspace/screen/tabs.less

@@ -5,7 +5,7 @@
     font-size: 12.5px;
 
     &:first-child {
-        border-radius: 12px 0px 0px 0px;
+        border-radius: 8px 0px 0px 0px;
     }
 
     &.color-green,

src/app/workspace/screen/tabs.tsx

@@ -7,56 +7,122 @@ import * as mobx from "mobx";
 import { sprintf } from "sprintf-js";
 import { boundMethod } from "autobind-decorator";
 import { For } from "tsx-control-statements/components";
-import cn from "classnames";
-import dayjs from "dayjs";
-import localizedFormat from "dayjs/plugin/localizedFormat";
 import { GlobalModel, GlobalCommandRunner, Session, Screen } from "../../../model/model";
-import { renderCmdText } from "../../common/common";
-import { ReactComponent as SquareIcon } from "../../assets/icons/tab/square.svg";
-import { ReactComponent as ActionsIcon } from "../../assets/icons/tab/actions.svg";
 import { ReactComponent as AddIcon } from "../../assets/icons/add.svg";
 import * as constants from "../../appconst";
 import { Reorder } from "framer-motion";
-import { MagicLayout } from "../../magiclayout";
+import { ScreenTab } from "./tab";
 
-import "../workspace.less";
 import "./tabs.less";
 
-dayjs.extend(localizedFormat);
-
 @mobxReact.observer
-class ScreenTabs extends React.Component<{ session: Session }, { showingScreens: Screen[] }> {
+class ScreenTabs extends React.Component<
+    { session: Session },
+    { showingScreens: Screen[]; scrollIntoViewTimeout: number }
+> {
     tabsRef: React.RefObject<any> = React.createRef();
     tabRefs: { [screenId: string]: React.RefObject<any> } = {};
     lastActiveScreenId: string = null;
     dragEndTimeout = null;
-    scrollIntoViewTimeout = null;
+    scrollIntoViewTimeoutId = null;
     deltaYHistory = [];
+    disposeScreensReaction = null;
 
     constructor(props: any) {
         super(props);
         this.state = {
             showingScreens: [],
+            scrollIntoViewTimeout: 0,
         };
     }
 
-    @mobx.computed
-    get activeScreenId(): string {
+    componentDidMount(): void {
+        // handle initial scrollIntoView
+        this.componentDidUpdate();
+
+        // populate showingScreens state
+        this.setState({ showingScreens: this.getScreens() });
+        // Update showingScreens state when the screens change
+        this.disposeScreensReaction = mobx.reaction(
+            () => this.getScreens(),
+            (screens) => {
+                // Different timeout for when screens are added vs removed
+                let timeout = 100;
+                if (screens.length < this.state.showingScreens.length) {
+                    timeout = 400;
+                }
+                this.setState({ showingScreens: screens, scrollIntoViewTimeout: timeout });
+            }
+        );
+
+        // Add the wheel event listener to the tabsRef
+        if (this.tabsRef.current) {
+            this.tabsRef.current.addEventListener("wheel", this.handleWheel, { passive: false });
+        }
+    }
+
+    componentWillUnmount() {
+        if (this.dragEndTimeout) {
+            clearTimeout(this.dragEndTimeout);
+        }
+
+        if (this.disposeScreensReaction) {
+            this.disposeScreensReaction(); // Clean up the reaction
+        }
+    }
+
+    componentDidUpdate(): void {
+        // Scroll the active screen into view
+        let activeScreenId = this.getActiveScreenId();
+        if (activeScreenId !== this.lastActiveScreenId) {
+            if (this.scrollIntoViewTimeoutId) {
+                clearTimeout(this.scrollIntoViewTimeoutId);
+            }
+            this.lastActiveScreenId = activeScreenId;
+            this.scrollIntoViewTimeoutId = setTimeout(() => {
+                if (!this.tabsRef.current) {
+                    return;
+                }
+                let tabElem = this.tabsRef.current.querySelector(
+                    sprintf('.screen-tab[data-screenid="%s"]', activeScreenId)
+                );
+                if (!tabElem) {
+                    return;
+                }
+                tabElem.scrollIntoView();
+            }, this.state.scrollIntoViewTimeout);
+        }
+    }
+
+    @boundMethod
+    getActiveScreenId(): string {
         let { session } = this.props;
         if (session) {
             return session.activeScreenId.get();
         }
+        return null;
     }
 
     @mobx.computed
-    get screens(): Screen[] {
-        if (this.activeScreenId) {
-            let screens = GlobalModel.getSessionScreens(this.props.session.sessionId);
-            let showingScreens = screens
-                .filter((screen) => !screen.archived.get() || this.activeScreenId === screen.screenId)
-                .sort((a, b) => a.screenIdx.get() - b.screenIdx.get());
-            return showingScreens;
+    @boundMethod
+    getScreens(): Screen[] {
+        let activeScreenId = this.getActiveScreenId();
+        if (!activeScreenId) {
+            return [];
         }
+
+        let screens = GlobalModel.getSessionScreens(this.props.session.sessionId);
+        let showingScreens = [];
+
+        for (const screen of screens) {
+            if (!screen.archived.get() || activeScreenId === screen.screenId) {
+                showingScreens.push(screen);
+            }
+        }
+
+        showingScreens.sort((a, b) => a.screenIdx.get() - b.screenIdx.get());
+
+        return showingScreens;
     }
 
     @boundMethod
@@ -103,68 +169,6 @@ class ScreenTabs extends React.Component<{ session: Session }, { showingScreens:
         // For touchpad events, do nothing and let the browser handle it
     }
 
-    componentDidMount(): void {
-        this.componentDidUpdate();
-
-        // Add the wheel event listener to the tabsRef
-        if (this.tabsRef.current) {
-            this.tabsRef.current.addEventListener("wheel", this.handleWheel, { passive: false });
-        }
-    }
-
-    componentWillUnmount() {
-        if (this.dragEndTimeout) {
-            clearTimeout(this.dragEndTimeout);
-            clearTimeout(this.scrollIntoViewTimeout);
-        }
-    }
-
-    componentDidUpdate(): void {
-        // Scroll the active screen into view
-        let { session } = this.props;
-        let activeScreenId = session.activeScreenId.get();
-        if (activeScreenId !== this.lastActiveScreenId) {
-            if (this.scrollIntoViewTimeout) {
-                clearTimeout(this.scrollIntoViewTimeout);
-            }
-            this.scrollIntoViewTimeout = setTimeout(() => {
-                if (this.tabsRef.current) {
-                    let tabElem = this.tabsRef.current.querySelector(
-                        sprintf('.screen-tab[data-screenid="%s"]', activeScreenId)
-                    );
-                    if (tabElem) {
-                        tabElem.scrollIntoView();
-                    }
-                }
-                this.lastActiveScreenId = activeScreenId;
-            }, 100);
-        }
-
-        // Set the showingScreens state if it's not set or if the number of screens has changed.
-        // Individual screen update are handled automatically by mobx.
-        if (this.screens && this.state.showingScreens.length !== this.screens.length) {
-            this.setState({ showingScreens: this.screens });
-        }
-    }
-
-    @boundMethod
-    handleDragEnd(screenId) {
-        if (this.dragEndTimeout) {
-            clearTimeout(this.dragEndTimeout);
-        }
-
-        // Wait for the animation to complete
-        this.dragEndTimeout = setTimeout(() => {
-            const tabElement = this.tabRefs[screenId].current;
-            const finalTabPosition = tabElement.offsetLeft;
-
-            // Calculate the new index based on the final position
-            const newIndex = Math.floor(finalTabPosition / MagicLayout.TabWidth);
-
-            GlobalCommandRunner.screenReorder(screenId, `${newIndex + 1}`);
-        }, 100);
-    }
-
     @boundMethod
     openScreenSettings(e: any, screen: Screen): void {
         e.preventDefault();
@@ -175,75 +179,6 @@ class ScreenTabs extends React.Component<{ session: Session }, { showingScreens:
         GlobalModel.modalsModel.pushModal(constants.SCREEN_SETTINGS);
     }
 
-    renderTabIcon = (screen: Screen): React.ReactNode => {
-        const tabIcon = screen.getTabIcon();
-        if (tabIcon === "default" || tabIcon === "square") {
-            return (
-                <div className="icon svg-icon">
-                    <SquareIcon className="left-icon" />
-                </div>
-            );
-        }
-        return (
-            <div className="icon fa-icon">
-                <i className={`fa-sharp fa-solid fa-${tabIcon}`}></i>
-            </div>
-        );
-    };
-
-    renderTab(screen: Screen, activeScreenId: string, index: number): JSX.Element {
-        let tabIndex = null;
-        if (index + 1 <= 9) {
-            tabIndex = <div className="tab-index">{renderCmdText(String(index + 1))}</div>;
-        }
-        let settings = (
-            <div onClick={(e) => this.openScreenSettings(e, screen)} title="Actions" className="tab-gear">
-                <ActionsIcon className="icon hoverEffect " />
-            </div>
-        );
-        let archived = screen.archived.get() ? (
-            <i title="archived" className="fa-sharp fa-solid fa-box-archive" />
-        ) : null;
-
-        let webShared = screen.isWebShared() ? (
-            <i title="shared to web" className="fa-sharp fa-solid fa-share-nodes web-share-icon" />
-        ) : null;
-
-        // Create a ref for the tab if it doesn't exist
-        if (!this.tabRefs[screen.screenId]) {
-            this.tabRefs[screen.screenId] = React.createRef();
-        }
-
-        return (
-            <Reorder.Item
-                ref={this.tabRefs[screen.screenId]}
-                value={screen}
-                id={"screen-" + screen.screenId}
-                whileDrag={{
-                    backgroundColor: "rgba(13, 13, 13, 0.85)",
-                }}
-                data-screenid={screen.screenId}
-                className={cn(
-                    "screen-tab",
-                    { "is-active": activeScreenId == screen.screenId, "is-archived": screen.archived.get() },
-                    "color-" + screen.getTabColor()
-                )}
-                onPointerDown={() => this.handleSwitchScreen(screen.screenId)}
-                onContextMenu={(event) => this.openScreenSettings(event, screen)}
-                onDragEnd={() => this.handleDragEnd(screen.screenId)}
-            >
-                {this.renderTabIcon(screen)}
-                <div className="tab-name truncate">
-                    {archived}
-                    {webShared}
-                    {screen.name.get()}
-                </div>
-                {tabIndex}
-                {settings}
-            </Reorder.Item>
-        );
-    }
-
     render() {
         let { showingScreens } = this.state;
         let { session } = this.props;
@@ -252,7 +187,8 @@ class ScreenTabs extends React.Component<{ session: Session }, { showingScreens:
         }
         let screen: Screen | null = null;
         let index = 0;
-        let activeScreenId = session.activeScreenId.get();
+        let activeScreenId = this.getActiveScreenId();
+
         return (
             <div className="screen-tabs-container">
                 <Reorder.Group
@@ -266,9 +202,13 @@ class ScreenTabs extends React.Component<{ session: Session }, { showingScreens:
                     values={showingScreens}
                 >
                     <For each="screen" index="index" of={showingScreens}>
-                        <React.Fragment key={screen.screenId}>
-                            {this.renderTab(screen, activeScreenId, index)}
-                        </React.Fragment>
+                        <ScreenTab
+                            key={screen.screenId}
+                            screen={screen}
+                            activeScreenId={activeScreenId}
+                            index={index}
+                            onSwitchScreen={this.handleSwitchScreen}
+                        />
                     </For>
                 </Reorder.Group>
                 <div key="new-screen" className="new-screen" onClick={this.handleNewScreen}>