simplify scrollbar code in terminal, simplify fit code (#2944)

Author: sawka

frontend/app/theme.scss

@@ -14,8 +14,9 @@
     --accent-color: rgb(88, 193, 66);
     --panel-bg-color: rgba(31, 33, 31, 0.5);
     --highlight-bg-color: rgba(255, 255, 255, 0.2);
-    --markdown-font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", "Noto Sans", Helvetica, Arial, sans-serif,
-        "Apple Color Emoji", "Segoe UI Emoji";
+    --markdown-font-family:
+        -apple-system, BlinkMacSystemFont, "Segoe UI", "Noto Sans", Helvetica, Arial, sans-serif, "Apple Color Emoji",
+        "Segoe UI Emoji";
     --markdown-font-size: 14px;
     --markdown-fixed-font-size: 12px;
     --error-color: rgb(229, 77, 46);
@@ -66,7 +67,6 @@
     --zindex-layout-ephemeral-node: 9;
     --zindex-block-mask-inner: 10;
     --zindex-app-background: -1;
-
     // z-indexes in xterm.css
     // xterm-helpers: 5
     // xterm-helper-textarea: -5
@@ -76,7 +76,6 @@
     // xterm-decoration-top-layer: 7
     // xterm-decoration-overview-ruler: 8
     // xterm-decoration-top: 2
-    --zindex-xterm-viewport-overlay: 5; // Viewport contains the scrollbar
 
     // modal colors
     --modal-bg-color: #232323;

frontend/app/view/term/fitaddon.ts

@@ -27,7 +27,7 @@ const MINIMUM_ROWS = 1;
 
 export class FitAddon implements ITerminalAddon, IFitApi {
     private _terminal: Terminal | undefined;
-    public noScrollbar: boolean = false;
+    public scrollbarWidth: number | null = null;
 
     public activate(terminal: Terminal): void {
         this._terminal = terminal;
@@ -68,12 +68,12 @@ export class FitAddon implements ITerminalAddon, IFitApi {
             return undefined;
         }
 
-        // UPDATED CODE (removed reliance on FALLBACK_SCROLL_BAR_WIDTH in viewport)
-        const measuredScrollBarWidth =
-            core.viewport._viewportElement.offsetWidth - core.viewport._scrollArea.offsetWidth;
-        let scrollbarWidth = this._terminal.options.scrollback === 0 ? 0 : measuredScrollBarWidth;
-        if (this.noScrollbar) {
-            scrollbarWidth = 0;
+        // UPDATED CODE (removed reliance on FALLBACK_SCROLL_BAR_WIDTH in viewport, allow just setting the scrollbar width when known)
+        let scrollbarWidth: number;
+        if (this.scrollbarWidth != null) {
+            scrollbarWidth = this.scrollbarWidth;
+        } else {
+            scrollbarWidth = core.viewport._viewportElement.offsetWidth - core.viewport._scrollArea.offsetWidth;
         }
         // END UPDATED CODE
 

frontend/app/view/term/term.scss

@@ -59,8 +59,7 @@
         min-height: 0;
         overflow: hidden;
         line-height: 1;
-        margin: 5px;
-        margin-left: 4px;
+        margin: 5px 1px 5px 4px;
     }
 
     .term-htmlelem {
@@ -126,30 +125,12 @@
         }
     }
 
-    // The 18px width is the width of the scrollbar plus the margin
-    .term-scrollbar-show-observer {
-        z-index: calc(var(--zindex-xterm-viewport-overlay) - 1);
-        position: absolute;
-        top: 0;
-        right: 0;
-        height: 100%;
-        width: 18px;
-    }
-
-    .term-scrollbar-hide-observer {
-        z-index: calc(var(--zindex-xterm-viewport-overlay) + 1);
-        display: none;
-        position: absolute;
-        top: 0;
-        left: 0;
-        height: 100%;
-        width: calc(100% - 18px);
-    }
-
     .terminal {
+        width: 100%;
+
         .xterm-viewport {
             &::-webkit-scrollbar {
-                width: 6px;
+                width: 6px; /* this needs to match fitAddon.scrollbarWidth in termwrap.ts */
                 height: 6px;
             }
 
@@ -158,24 +139,23 @@
             }
 
             &::-webkit-scrollbar-thumb {
-                display: none;
-                background-color: var(--scrollbar-thumb-color);
+                background-color: transparent;
                 border-radius: 4px;
                 margin: 0 1px 0 1px;
 
                 &:hover {
-                    background-color: var(--scrollbar-thumb-hover-color);
+                    background-color: var(--scrollbar-thumb-hover-color) !important;
                 }
 
                 &:active {
-                    background-color: var(--scrollbar-thumb-active-color);
+                    background-color: var(--scrollbar-thumb-active-color) !important;
                 }
             }
         }
 
         &:hover {
             .xterm-viewport::-webkit-scrollbar-thumb {
-                display: block;
+                background-color: var(--scrollbar-thumb-color);
             }
         }
     }

frontend/app/view/term/term.tsx

@@ -354,18 +354,6 @@ const TerminalView = ({ blockId, model }: ViewComponentProps<TermViewModel>) =>
         }
     }, [isMI, isBasicTerm, isFocused]);
 
-    const scrollbarHideObserverRef = React.useRef<HTMLDivElement>(null);
-    const onScrollbarShowObserver = React.useCallback(() => {
-        const termViewport = viewRef.current.getElementsByClassName("xterm-viewport")[0] as HTMLDivElement;
-        termViewport.style.zIndex = "var(--zindex-xterm-viewport-overlay)";
-        scrollbarHideObserverRef.current.style.display = "block";
-    }, []);
-    const onScrollbarHideObserver = React.useCallback(() => {
-        const termViewport = viewRef.current.getElementsByClassName("xterm-viewport")[0] as HTMLDivElement;
-        termViewport.style.zIndex = "auto";
-        scrollbarHideObserverRef.current.style.display = "none";
-    }, []);
-
     const stickerConfig = {
         charWidth: 8,
         charHeight: 16,
@@ -388,20 +376,13 @@ const TerminalView = ({ blockId, model }: ViewComponentProps<TermViewModel>) =>
 
     return (
         <div className={clsx("view-term", "term-mode-" + termMode)} ref={viewRef} onContextMenu={handleContextMenu}>
-            {termBg && <div className="absolute inset-0 z-0 pointer-events-none" style={termBg} />}
+            {termBg && <div key="term-bg" className="absolute inset-0 z-0 pointer-events-none" style={termBg} />}
             <TermResyncHandler blockId={blockId} model={model} />
             <TermThemeUpdater blockId={blockId} model={model} termRef={model.termRef} />
             <TermStickers config={stickerConfig} />
             <TermToolbarVDomNode key="vdom-toolbar" blockId={blockId} model={model} />
             <TermVDomNode key="vdom" blockId={blockId} model={model} />
-            <div key="conntectElem" className="term-connectelem" ref={connectElemRef}>
-                <div className="term-scrollbar-show-observer" onPointerOver={onScrollbarShowObserver} />
-                <div
-                    ref={scrollbarHideObserverRef}
-                    className="term-scrollbar-hide-observer"
-                    onPointerOver={onScrollbarHideObserver}
-                />
-            </div>
+            <div key="connect-elem" className="term-connectelem" ref={connectElemRef} />
             <NullErrorBoundary debugName="TermLinkTooltip">
                 <TermLinkTooltip termWrap={termWrapInst} />
             </NullErrorBoundary>

frontend/app/view/term/termwrap.ts

@@ -129,7 +129,7 @@ export class TermWrap {
         this.lastCommandAtom = jotai.atom(null) as jotai.PrimitiveAtom<string | null>;
         this.terminal = new Terminal(options);
         this.fitAddon = new FitAddon();
-        this.fitAddon.noScrollbar = PLATFORM === PlatformMacOS;
+        this.fitAddon.scrollbarWidth = 6; // this needs to match scrollbar width in term.scss
         this.serializeAddon = new SerializeAddon();
         this.searchAddon = new SearchAddon();
         this.terminal.loadAddon(this.searchAddon);