chore: prep for code review

Signed-off-by: Scott Dover <[email protected]>

Author: Scott Dover

client/src/connection/itc/ItcLibraryAdapter.ts

@@ -127,7 +127,7 @@ class ItcLibraryAdapter implements LibraryAdapter {
           }
         : {};
 
-    const { rows } = await this.getRows(item, start, limit);
+    const { rows } = await this.getRows(item, start, limit, []);
 
     rows.unshift(columns);
 

client/src/connection/itc/script.ts

@@ -258,7 +258,7 @@ class SASRunner{
       $this.dataConnection, # Use the active connection
       3,  # adOpenStatic
       1,  # adLockReadOnly
-      1 # adCmdTableDirect
+      1   # adCmdText
     )
 
     $records = [List[List[object]]]::new()

client/src/webview/ColumnHeader.tsx

@@ -48,6 +48,8 @@ const ColumnHeader = ({
     column.sort && currentSortedColumns.length > 1
       ? `${column.sortIndex + 1}`
       : "";
+  const dropdownClassname =
+    currentColumn?.colId === column.colId ? "active dropdown" : "dropdown";
 
   const applyColumnState = (state: ColumnState[]) => {
     api.applyColumnState({ state, defaultState: { sort: null } });
@@ -104,38 +106,20 @@ const ColumnHeader = ({
 
   return (
     <div className={`ag-cell-label-container ${theme}`} role="presentation">
-      <div
-        data-ref="eLabel"
-        className="ag-header-cell-label"
-        role="presentation"
-      >
-        <span
-          className={`header-icon ${getIconForColumnType(columnType)}`}
-        ></span>
+      <div className="ag-header-cell-label" role="presentation">
+        <span className={`header-icon ${getIconForColumnType(columnType)}`} />
         <span className="ag-header-cell-text">{column.colId}</span>
-
         <span className="sort-icon-wrapper">
           {!!column.sort && (
             <>
-              <span
-                className={`sort-icon ${column.sort === "asc" ? "ascending" : "descending"}`}
-              ></span>
-              {!!columnNumber && (
-                <span className="sort-number">{columnNumber}</span>
-              )}
+              <span className={`icon ${column.sort}`}></span>
+              {!!columnNumber && <span className="number">{columnNumber}</span>}
             </>
           )}
         </span>
-
-        <div
-          className={
-            currentColumn?.colId === column.colId
-              ? "active dropdown"
-              : "dropdown"
-          }
-        >
+        <div className={dropdownClassname}>
           <button ref={ref} type="button" onClick={displayColumnMenu}>
-            <span></span>
+            <span />
           </button>
         </div>
       </div>

client/src/webview/ColumnHeaderMenu.tsx

@@ -1,7 +1,6 @@
 // Copyright © 2025, SAS Institute Inc., Cary, NC, USA.  All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
-
-import { Fragment, useEffect, useRef, useState } from "react";
+import { useEffect, useRef, useState } from "react";
 
 import { AgColumn } from "ag-grid-community";
 
@@ -19,41 +18,50 @@ export interface ColumnHeaderProps {
 }
 
 interface MenuItem {
-  name: string;
   checked?: boolean;
-  onPress?: () => void;
   children?: (MenuItem | string)[];
   disabled?: boolean;
+  name: string;
+  onPress?: () => void;
 }
 
 const GridMenu = ({
+  left: incomingLeft,
   menuItems,
+  parentDimensions,
+  subMenu,
   theme,
   top,
-  left: incomingLeft,
-  subMenu,
-  parentDimensions,
 }: {
+  left?: number;
   menuItems: (MenuItem | string)[];
+  parentDimensions?: { left: number; width: number };
+  subMenu?: boolean;
   theme: string;
   top: number;
-  left?: number;
-  subMenu?: boolean;
-  parentDimensions?: { left: number; width: number };
 }) => {
   const menuRef = useRef<HTMLDivElement>(undefined);
+  const [activeIndex, setActiveIndex] = useState(-1);
   const [subMenuItems, setSubMenuItems] = useState<(MenuItem | string)[]>([]);
   const className = subMenu
     ? `ag-menu ag-ltr ag-popup-child ${theme}`
     : `ag-menu ag-column-menu ag-ltr ag-popup-child ag-popup-positioned-under ${theme}`;
-  const [activeIndex, setActiveIndex] = useState(-1);
 
+  // The following useEffect positions our column header menu. There are three general
+  // ways of laying things out.
+  // - option 1. If there is enough room for the parent menu and child menu to the right, the
+  //.  menus are displayed left to right.
+  // - option 2. If the parent menu has enough room, we don't shift it. If the child menu doesn't
+  //.  fit to the right, we move it to the left side.
+  // - option 3. The parent menu doesn't fit on the screen, so we shift it to the left and
+  //   put the child menu on the left side.
   const [left, setLeft] = useState(parentDimensions?.left ?? incomingLeft);
   const [displayed, setDisplayed] = useState(false);
   useEffect(() => {
     const clientWidth = menuRef.current.closest("body").clientWidth;
     const width = menuRef.current.getBoundingClientRect().width;
 
+    setDisplayed(true);
     if (parentDimensions) {
       // First, lets put the child menu to the right
       let adjustedLeft = parentDimensions.left + parentDimensions.width;
@@ -62,17 +70,15 @@ const GridMenu = ({
         adjustedLeft = parentDimensions.left - width;
       }
       setLeft(adjustedLeft);
-      setDisplayed(true);
       return;
     }
     if (left + width > clientWidth) {
       setLeft(left - (left + width - clientWidth + 15));
     }
-    setDisplayed(true);
-  }, []);
+  }, []); // eslint-disable-line react-hooks/exhaustive-deps
 
   return (
-    <Fragment>
+    <>
       {subMenuItems.length > 0 && (
         <GridMenu
           menuItems={subMenuItems}
@@ -106,19 +112,7 @@ const GridMenu = ({
             ></div>
             {menuItems.map((menuItem, index) => {
               if (typeof menuItem === "string") {
-                return (
-                  <div
-                    className="ag-menu-separator"
-                    aria-hidden="true"
-                    key={index}
-                  >
-                    {" "}
-                    <div className="ag-menu-separator-part"></div>{" "}
-                    <div className="ag-menu-separator-part"></div>{" "}
-                    <div className="ag-menu-separator-part"></div>{" "}
-                    <div className="ag-menu-separator-part"></div>{" "}
-                  </div>
-                );
+                return <Separator key={index} />;
               }
               return (
                 <div
@@ -146,6 +140,7 @@ const GridMenu = ({
                     setActiveIndex(-1);
                     const targetInPopup = Array.from(
                       document.querySelectorAll(".ag-popup"),
+                      // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
                     ).some((t) => t.contains(e.target as HTMLElement));
                     if (!targetInPopup) {
                       setSubMenuItems([]);
@@ -154,20 +149,17 @@ const GridMenu = ({
                 >
                   <span
                     className="ag-menu-option-part ag-menu-option-icon"
-                    data-ref="eIcon"
                     role="presentation"
                   >
                     {menuItem.checked && (
                       <span
                         className="ag-icon ag-icon-tick"
                         role="presentation"
-                        unselectable="on"
-                      ></span>
+                      />
                     )}
                   </span>
                   <span
                     className="ag-menu-option-part ag-menu-option-text"
-                    data-ref="eName"
                     onClick={() => {
                       if (menuItem.disabled) {
                         return;
@@ -182,20 +174,13 @@ const GridMenu = ({
                   >
                     {menuItem.name}
                   </span>
-                  <span
-                    className="ag-menu-option-part ag-menu-option-shortcut"
-                    data-ref="eShortcut"
-                  ></span>
+                  <span className="ag-menu-option-part ag-menu-option-shortcut"></span>
                   {menuItem.children && menuItem.children.length > 0 && (
-                    <span
-                      className="ag-menu-option-part ag-menu-option-popup-pointer"
-                      data-ref="ePopupPointer"
-                    >
+                    <span className="ag-menu-option-part ag-menu-option-popup-pointer">
                       <span
                         className="ag-icon ag-icon-small-right"
                         role="presentation"
-                        unselectable="on"
-                      ></span>
+                      />
                     </span>
                   )}
                 </div>
@@ -204,11 +189,11 @@ const GridMenu = ({
             <div
               className="ag-tab-guard ag-tab-guard-bottom"
               role="presentation"
-            ></div>
+            />
           </div>
         </div>
       </div>
-    </Fragment>
+    </>
   );
 };
 
@@ -226,13 +211,13 @@ const ColumnHeaderMenu = ({
 }: ColumnHeaderProps) => {
   const menuItems = [
     {
-      name: t["Sort"],
+      name: t.Sort,
       children: [
         {
           name:
             hasSort && !column.sort
               ? t["Ascending (add to sorting)"]
-              : t["Ascending"],
+              : t.Ascending,
           checked: column.sort === "asc",
           onPress: () => {
             sortColumn("asc");
@@ -243,7 +228,7 @@ const ColumnHeaderMenu = ({
           name:
             hasSort && !column.sort
               ? t["Descending (add to sorting)"]
-              : t["Descending"],
+              : t.Descending,
           checked: column.sort === "desc",
           onPress: () => {
             sortColumn("desc");
@@ -274,4 +259,14 @@ const ColumnHeaderMenu = ({
   return <GridMenu menuItems={menuItems} top={top} left={left} theme={theme} />;
 };
 
+const Separator = () => (
+  <div className="ag-menu-separator" aria-hidden="true">
+    {" "}
+    <div className="ag-menu-separator-part"></div>{" "}
+    <div className="ag-menu-separator-part"></div>{" "}
+    <div className="ag-menu-separator-part"></div>{" "}
+    <div className="ag-menu-separator-part"></div>{" "}
+  </div>
+);
+
 export default ColumnHeaderMenu;

client/src/webview/DataViewer.css

@@ -126,30 +126,30 @@ button,
   margin-left: auto;
 }
 
-.sort-icon-wrapper .sort-icon,
-.sort-icon-wrapper .sort-number {
+.sort-icon-wrapper .icon,
+.sort-icon-wrapper .number {
   width: 16px;
   height: 16px;
   display: inline-block;
   vertical-align: middle;
 }
 
-.sort-icon-wrapper .sort-icon.ascending {
+.sort-icon-wrapper .icon.asc {
   background: url(../../../icons/light/arrow-up.svg);
   background-size: 16px 16px;
 }
 
-.vscode-dark .sort-icon-wrapper .sort-icon.ascending {
+.vscode-dark .sort-icon-wrapper .icon.asc {
   background: url(../../../icons/dark/arrow-up.svg);
   background-size: 16px 16px;
 }
 
-.sort-icon-wrapper .sort-icon.descending {
+.sort-icon-wrapper .icon.desc {
   background: url(../../../icons/light/arrow-down.svg);
   background-size: 16px 16px;
 }
 
-.vscode-dark .sort-icon-wrapper .sort-icon.descending {
+.vscode-dark .sort-icon-wrapper .icon.desc {
   background: url(../../../icons/dark/arrow-down.svg);
   background-size: 16px 16px;
 }

client/src/webview/useDataViewer.tsx

@@ -183,7 +183,7 @@ const useDataViewer = (theme: string) => {
 
       setColumns(columns);
     });
-  }, [columns.length]);
+  }, [columns.length, theme]);
 
   useEffect(() => {
     window.addEventListener("contextmenu", contextMenuHandler, true);