Add linting rule to detect when icons are used in buttons (#19104)

* Add linting rule to detect when icons are used in buttons

* Update docs for links

* Add lint for link

eslint.config.mjs

@@ -208,6 +208,7 @@ export default tseslint.config(
         { ignoreIfHas: ["bitPasswordInputToggle"] },
       ],
       "@bitwarden/components/no-bwi-class-usage": "warn",
+      "@bitwarden/components/no-icon-children-in-bit-button": "warn",
     },
   },
 

libs/components/src/link/link.mdx

@@ -1,4 +1,12 @@
-import { Meta, Story, Primary, Controls, Title, Description } from "@storybook/addon-docs/blocks";
+import {
+  Meta,
+  Story,
+  Canvas,
+  Primary,
+  Controls,
+  Title,
+  Description,
+} from "@storybook/addon-docs/blocks";
 
 import * as stories from "./link.stories";
 
@@ -33,15 +41,25 @@ You can use one of the following variants by providing it as the `linkType` inpu
 If you want to display a link with a smaller text size, apply the `tw-text-sm` class. This will
 match the `body2` variant of the Typography directive.
 
-## With icons
+## With Icon
 
-Text Links/buttons can have icons on left or the right.
+Use the `startIcon` and `endIcon` inputs to add a Bitwarden icon (`bwi-*`) before or after the link
+label. Do not use a `<bit-icon>` component inside the link as this may not have the correct styling
+and spacing.
 
-To indicate a new or add action, the <i class="bwi bwi-plus-circle"></i> icon on is used on the
-left.
+### Icon before the label
 
-An angle icon, <i class="bwi bwi-angle-right"></i>, is used on the left to indicate an expand to
-show/hide additional content.
+```html
+<a bitLink startIcon="bwi-plus-circle">Add item</a>
+```
+
+### Icon after the label
+
+```html
+<a bitLink endIcon="bwi-angle-right">Next</a>
+```
+
+<Canvas of={stories.WithIcons} />
 
 ## Accessibility
 

libs/eslint/components/index.mjs

@@ -1,11 +1,13 @@
 import requireLabelOnBiticonbutton from "./require-label-on-biticonbutton.mjs";
 import requireThemeColorsInSvg from "./require-theme-colors-in-svg.mjs";
 import noBwiClassUsage from "./no-bwi-class-usage.mjs";
+import noIconChildrenInBitButton from "./no-icon-children-in-bit-button.mjs";
 
 export default {
   rules: {
     "require-label-on-biticonbutton": requireLabelOnBiticonbutton,
     "require-theme-colors-in-svg": requireThemeColorsInSvg,
     "no-bwi-class-usage": noBwiClassUsage,
+    "no-icon-children-in-bit-button": noIconChildrenInBitButton,
   },
 };

libs/eslint/components/no-icon-children-in-bit-button.mjs

@@ -0,0 +1,74 @@
+export const errorMessage =
+  'Avoid placing icon elements (<i class="bwi ..."> or <bit-icon>) inside a bitButton or bitLink. ' +
+  "Use the [startIcon] or [endIcon] inputs instead. " +
+  'Example: <button bitButton startIcon="bwi-plus">Label</button>';
+
+export default {
+  meta: {
+    type: "suggestion",
+    docs: {
+      description:
+        "Discourage using icon child elements inside bitButton; use startIcon/endIcon inputs instead",
+      category: "Best Practices",
+      recommended: true,
+    },
+    schema: [],
+  },
+  create(context) {
+    return {
+      Element(node) {
+        if (node.name !== "button" && node.name !== "a") {
+          return;
+        }
+
+        const allAttrNames = [
+          ...(node.attributes?.map((attr) => attr.name) ?? []),
+          ...(node.inputs?.map((input) => input.name) ?? []),
+        ];
+
+        if (!allAttrNames.includes("bitButton") && !allAttrNames.includes("bitLink")) {
+          return;
+        }
+
+        for (const child of node.children ?? []) {
+          if (!child.name) {
+            continue;
+          }
+
+          // <bit-icon> child
+          if (child.name === "bit-icon") {
+            context.report({
+              node: child,
+              message: errorMessage,
+            });
+            continue;
+          }
+
+          // <i> child with bwi class
+          if (child.name === "i") {
+            const classAttrs = [
+              ...(child.attributes?.filter((attr) => attr.name === "class") ?? []),
+              ...(child.inputs?.filter((input) => input.name === "class") ?? []),
+            ];
+
+            for (const classAttr of classAttrs) {
+              const classValue = classAttr.value || "";
+
+              if (typeof classValue !== "string") {
+                continue;
+              }
+
+              if (/\bbwi\b/.test(classValue)) {
+                context.report({
+                  node: child,
+                  message: errorMessage,
+                });
+                break;
+              }
+            }
+          }
+        }
+      },
+    };
+  },
+};

libs/eslint/components/no-icon-children-in-bit-button.spec.mjs

@@ -0,0 +1,97 @@
+import { RuleTester } from "@typescript-eslint/rule-tester";
+
+import rule, { errorMessage } from "./no-icon-children-in-bit-button.mjs";
+
+const ruleTester = new RuleTester({
+  languageOptions: {
+    parser: require("@angular-eslint/template-parser"),
+  },
+});
+
+ruleTester.run("no-icon-children-in-bit-button", rule.default, {
+  valid: [
+    {
+      name: "should allow bitButton with startIcon input",
+      code: `<button bitButton startIcon="bwi-plus">Add</button>`,
+    },
+    {
+      name: "should allow bitButton with endIcon input",
+      code: `<button bitButton endIcon="bwi-external-link">Open</button>`,
+    },
+    {
+      name: "should allow a[bitButton] with startIcon input",
+      code: `<a bitButton startIcon="bwi-external-link" href="https://example.com">Link</a>`,
+    },
+    {
+      name: "should allow <i> with bwi inside a regular button (no bitButton)",
+      code: `<button type="button"><i class="bwi bwi-lock"></i> Lock</button>`,
+    },
+    {
+      name: "should allow <bit-icon> inside a regular div",
+      code: `<div><bit-icon name="bwi-lock"></bit-icon></div>`,
+    },
+    {
+      name: "should allow bitButton with only text content",
+      code: `<button bitButton buttonType="primary">Save</button>`,
+    },
+    {
+      name: "should allow <i> without bwi class inside bitButton",
+      code: `<button bitButton><i class="fa fa-lock"></i> Lock</button>`,
+    },
+    {
+      name: "should allow bitLink with startIcon input",
+      code: `<a bitLink startIcon="bwi-external-link" href="https://example.com">Link</a>`,
+    },
+    {
+      name: "should allow bitLink with only text content",
+      code: `<a bitLink href="https://example.com">Link</a>`,
+    },
+  ],
+  invalid: [
+    {
+      name: "should warn on <i> with bwi class inside button[bitButton]",
+      code: `<button bitButton buttonType="primary"><i class="bwi bwi-plus"></i> Add</button>`,
+      errors: [{ message: errorMessage }],
+    },
+    {
+      name: "should warn on <i> with bwi class and extra classes inside button[bitButton]",
+      code: `<button bitButton><i class="bwi bwi-lock tw-me-2" aria-hidden="true"></i> Lock</button>`,
+      errors: [{ message: errorMessage }],
+    },
+    {
+      name: "should warn on <i> with bwi class inside a[bitButton]",
+      code: `<a bitButton buttonType="secondary"><i class="bwi bwi-external-link"></i> Link</a>`,
+      errors: [{ message: errorMessage }],
+    },
+    {
+      name: "should warn on <bit-icon> inside button[bitButton]",
+      code: `<button bitButton buttonType="primary"><bit-icon name="bwi-lock"></bit-icon> Lock</button>`,
+      errors: [{ message: errorMessage }],
+    },
+    {
+      name: "should warn on <bit-icon> inside a[bitButton]",
+      code: `<a bitButton><bit-icon name="bwi-clone"></bit-icon> Copy</a>`,
+      errors: [{ message: errorMessage }],
+    },
+    {
+      name: "should warn on multiple icon children inside bitButton",
+      code: `<button bitButton><i class="bwi bwi-plus"></i> Add <i class="bwi bwi-angle-down"></i></button>`,
+      errors: [{ message: errorMessage }, { message: errorMessage }],
+    },
+    {
+      name: "should warn on both <i> and <bit-icon> children",
+      code: `<button bitButton><i class="bwi bwi-plus"></i><bit-icon name="bwi-lock"></bit-icon></button>`,
+      errors: [{ message: errorMessage }, { message: errorMessage }],
+    },
+    {
+      name: "should warn on <i> with bwi class inside a[bitLink]",
+      code: `<a bitLink><i class="bwi bwi-external-link"></i> Link</a>`,
+      errors: [{ message: errorMessage }],
+    },
+    {
+      name: "should warn on <bit-icon> inside button[bitLink]",
+      code: `<button bitLink><bit-icon name="bwi-lock"></bit-icon> Lock</button>`,
+      errors: [{ message: errorMessage }],
+    },
+  ],
+});