Remove Dynamically Created Inline Styles from Map Layers (#1463)

* init commit

* change, add check-inline-styles script

* glob search, prettier, class selectors

* Apply suggestions from copilot code review

Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: andremig-bentley <[email protected]>
Co-authored-by: Copilot <[email protected]>

Author: andremig-bentley

change/@itwin-map-layers-5dadb923-d113-4d34-98b5-0abb350f618d.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Remove Dynamically Created Inline Styles",
+  "packageName": "@itwin/map-layers",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

package.json

@@ -19,6 +19,7 @@
     "change": "beachball change --no-commit",
     "change:dev": "beachball change --config beachball.config.dev.js --no-commit",
     "cspell": "cspell .",
+    "check-inline-styles": "node scripts/check-inline-styles.js",
     "publish": "beachball publish --new",
     "bump": "beachball bump",
     "prettier": "prettier --check .",
@@ -31,6 +32,7 @@
     "beachball": "^2.54.0",
     "cpx2": "^7.0.2",
     "cspell": "^9.2.0",
+    "glob": "^11.0.0",
     "lage": "^2.14.6",
     "prettier": "^3.6.2",
     "yargs": "^17.7.2"

packages/itwin/map-layers/src/ui/widget/MapLayerDroppable.scss

@@ -10,3 +10,18 @@
 .map-manager-display-none {
   display: none;
 }
+
+// Hover-based visibility for settings menus
+.map-manager-source-item {
+  .map-layer-settings-menu-wrapper,
+  .map-layer-settings-sublayers-menu {
+    visibility: hidden;
+  }
+
+  &:hover {
+    .map-layer-settings-menu-wrapper,
+    .map-layer-settings-sublayers-menu {
+      visibility: visible;
+    }
+  }
+}

packages/itwin/map-layers/src/ui/widget/MapLayerDroppable.tsx

@@ -58,11 +58,6 @@ const StrictModeDroppable = ({ children, ...props }: DroppableProps) =>{
   return <Droppable {...props}>{children}</Droppable>;
 };
 
-const changeVisibilityByElementId = (element: Element | null, visible: boolean) => {
-  if (element) {
-    element.setAttribute("style", `visibility: ${visible ? "visible" : "hidden"}`);
-  }
-};
 
 /** @internal */
 export function MapLayerDroppable(props: MapLayerDroppableProps) {
@@ -116,11 +111,6 @@ export function MapLayerDroppable(props: MapLayerDroppableProps) {
     [props],
   );
 
-  const changeSettingsMenuVisibility = (event: React.MouseEvent<HTMLDivElement, MouseEvent>, visible: boolean) => {
-    changeVisibilityByElementId(event.currentTarget.querySelector("#MapLayerSettingsMenuWrapper"), visible);
-    changeVisibilityByElementId(event.currentTarget.querySelector("#MapLayerSettingsSubLayersMenu"), visible);
-  };
-
   const renderItem: DraggableChildrenFn = (dragProvided, _, rubric) => {
     assert(props.layersList !== undefined);
     const activeLayer = props.layersList[rubric.source.index];
@@ -133,8 +123,6 @@ export function MapLayerDroppable(props: MapLayerDroppableProps) {
         key={activeLayer.name}
         {...dragProvided.draggableProps}
         ref={dragProvided.innerRef}
-        onMouseEnter={(event) => changeSettingsMenuVisibility(event, true)}
-        onMouseLeave={(event) => changeSettingsMenuVisibility(event, false)}
       >
         {/* Checkbox */}
         <Checkbox
@@ -204,7 +192,7 @@ export function MapLayerDroppable(props: MapLayerDroppableProps) {
         </span>
 
         {/* SubLayersPopupButton */}
-        <div id="MapLayerSettingsSubLayersMenu" className="map-manager-item-sub-layer-container map-manager-hidden">
+        <div className="map-manager-item-sub-layer-container map-layer-settings-sublayers-menu">
           {activeLayer.subLayers && activeLayer.subLayers.length > 1 && (
             <SubLayersPopupButton
               checkboxStyle="eye"
@@ -252,7 +240,7 @@ export function MapLayerDroppable(props: MapLayerDroppableProps) {
             <SvgStatusWarning />
           </IconButton>
         )}
-        <div id="MapLayerSettingsMenuWrapper" className="map-manager-hidden">
+        <div className="map-layer-settings-menu-wrapper">
           <MapLayerSettingsMenu
             activeViewport={props.activeViewport}
             mapLayerSettings={activeLayer}

pnpm-lock.yaml

@@ -0,0 +1,73 @@
+#!/usr/bin/env node
+
+/**
+ * Script to check for inline styles
+ * This validates that the map-layers package contains no inline style attributes
+ */
+
+const fs = require("fs");
+const path = require("path");
+const { glob } = require("glob");
+
+const mapLayersPath = "packages/itwin/map-layers/**/*.{ts,tsx}";
+
+async function checkForInlineStyles() {
+  try {
+    const files = await glob(mapLayersPath, { cwd: process.cwd() });
+    let violations = [];
+
+    files.forEach((file) => {
+      const content = fs.readFileSync(file, "utf8");
+      const lines = content.split("\n");
+
+      lines.forEach((line, index) => {
+        // Check for style={{ or style = { patterns
+        if (/style\s*=\s*\{/.test(line)) {
+          violations.push({
+            file,
+            line: index + 1,
+            content: line.trim(),
+          });
+        }
+
+        // Check for setAttribute("style", ...) patterns
+        if (/\.setAttribute\s*\(\s*["']style["']\s*,/.test(line)) {
+          violations.push({
+            file,
+            line: index + 1,
+            content: line.trim(),
+          });
+        }
+
+        // Check for .style.property = value patterns
+        if (/\.style\.[a-zA-Z][a-zA-Z0-9]*\s*=/.test(line)) {
+          violations.push({
+            file,
+            line: index + 1,
+            content: line.trim(),
+          });
+        }
+      });
+    });
+
+    if (violations.length > 0) {
+      console.error("\nInline styles detected in map-layers package:");
+      violations.forEach((v) => {
+        console.error(`In file: ${v.file}`);
+        console.error(`On line ${v.line}: ${v.content}`);
+        console.error("");
+      });
+      console.error(`Total violations: ${violations.length}`);
+      console.error("Please convert inline styles to CSS classes.");
+    } else {
+      console.log("\nNo inline styles found in map-layers package");
+      console.log(`Checked ${files.length} files`);
+    }
+  } catch (error) {
+    console.error("Error checking for inline styles:", error);
+    process.exit(1);
+  }
+}
+
+// Run the check
+checkForInlineStyles();