diff --git a/libs/components/src/icon/icon.component.ts b/libs/components/src/icon/icon.component.ts index c2dc468dc71..c1ec43d5bd0 100644 --- a/libs/components/src/icon/icon.component.ts +++ b/libs/components/src/icon/icon.component.ts @@ -1,4 +1,10 @@ -import { ChangeDetectionStrategy, Component, computed, input } from "@angular/core"; +import { + booleanAttribute, + ChangeDetectionStrategy, + Component, + computed, + input, +} from "@angular/core"; import { BitwardenIcon } from "../shared/icon"; @@ -24,7 +30,12 @@ export class IconComponent { */ readonly ariaLabel = input(); - protected readonly classList = computed(() => { - return ["bwi", this.name()].join(" "); - }); + /** + * Whether the icon should use a fixed width + */ + readonly fixedWidth = input(false, { transform: booleanAttribute }); + + protected readonly classList = computed(() => + ["bwi", this.name(), this.fixedWidth() && "bwi-fw"].filter(Boolean), + ); } diff --git a/libs/components/src/icon/icon.mdx b/libs/components/src/icon/icon.mdx index 0914d681e59..f52a3b99626 100644 --- a/libs/components/src/icon/icon.mdx +++ b/libs/components/src/icon/icon.mdx @@ -15,7 +15,7 @@ The `bit-icon` component renders Bitwarden Web Icons (bwi) using icon font class ## Basic Usage ```html - + ``` ## Icon Names @@ -29,7 +29,7 @@ By default, icons are decorative and marked with `aria-hidden="true"`. To make a provide an `ariaLabel`: ```html - + ``` ## Styling @@ -38,7 +38,7 @@ The component renders as an inline element. Apply standard CSS classes or styles appearance: ```html - + ``` ## Note on SVG Icons diff --git a/libs/components/src/icon/icon.stories.ts b/libs/components/src/icon/icon.stories.ts index 5626407ea51..dea5ce390ad 100644 --- a/libs/components/src/icon/icon.stories.ts +++ b/libs/components/src/icon/icon.stories.ts @@ -54,6 +54,47 @@ export const WithAriaLabel: Story = { }, }; +export const FixedWidth: Story = { + args: { + name: "bwi-lock", + fixedWidth: true, + }, +}; + +export const FixedWidthComparison: Story = { + render: () => ({ + template: ` +
+
+ + bwi-lock (fixed width) +
+
+ + bwi-eye (fixed width) +
+
+ + bwi-collection (fixed width) +
+
+
+ + bwi-lock (default) +
+
+ + bwi-eye (default) +
+
+ + bwi-collection (default) +
+
+ `, + }), +}; + export const CompareWithLegacy: Story = { render: () => ({ template: ` `, diff --git a/libs/eslint/components/bwi-utils.mjs b/libs/eslint/components/bwi-utils.mjs new file mode 100644 index 00000000000..c3375d1f98c --- /dev/null +++ b/libs/eslint/components/bwi-utils.mjs @@ -0,0 +1,30 @@ +/** Matches any bwi class: the bare "bwi" base class or "bwi-" variants. */ +export const BWI_CLASS_RE = /\bbwi(?:-[\w-]+)?\b/g; + +/** + * Helper / utility classes from libs/angular/src/scss/bwicons/styles/style.scss. + * These don't represent icon names and can be used independently. + */ +export const BWI_HELPER_CLASSES = new Set([ + "bwi-fw", // Fixed width + "bwi-sm", // Small + "bwi-lg", // Large + "bwi-2x", // 2x size + "bwi-3x", // 3x size + "bwi-4x", // 4x size + "bwi-spin", // Spin animation + "bwi-ul", // List + "bwi-li", // List item + "bwi-rotate-270", // Rotation +]); + +/** + * Extract the icon name from a class string. + * Returns the first bwi-* class that is not a helper class and not the bare "bwi" base class. + * Returns null if no single icon name can be determined. + */ +export function extractIconNameFromClassValue(classValue) { + const bwiClasses = classValue.match(BWI_CLASS_RE) || []; + const iconNames = bwiClasses.filter((cls) => cls !== "bwi" && !BWI_HELPER_CLASSES.has(cls)); + return iconNames.length === 1 ? iconNames[0] : null; +} diff --git a/libs/eslint/components/no-bwi-class-usage.mjs b/libs/eslint/components/no-bwi-class-usage.mjs index 6f856646a07..4f5057f810b 100644 --- a/libs/eslint/components/no-bwi-class-usage.mjs +++ b/libs/eslint/components/no-bwi-class-usage.mjs @@ -1,33 +1,113 @@ +import { BWI_CLASS_RE, BWI_HELPER_CLASSES, extractIconNameFromClassValue } from "./bwi-utils.mjs"; + export const errorMessage = "Use component instead of applying 'bwi' classes directly. Example: "; -// Helper classes from libs/angular/src/scss/bwicons/styles/style.scss -// These are utility classes that can be used independently -const ALLOWED_BWI_HELPER_CLASSES = new Set([ - "bwi-fw", // Fixed width - "bwi-sm", // Small - "bwi-lg", // Large - "bwi-2x", // 2x size - "bwi-3x", // 3x size - "bwi-4x", // 4x size - "bwi-spin", // Spin animation - "bwi-ul", // List - "bwi-li", // List item - "bwi-rotate-270", // Rotation -]); +/** + * Parse the class string and return the remaining classes and whether bwi-fw is present. + * Drops: "bwi" base class, the icon name class, and "bwi-fw" (mapped to fixedWidth input). + * Keeps: other helper bwi classes and non-bwi classes (tw-*, etc.) + */ +function parseClasses(classValue, iconName) { + let hasFixedWidth = false; + const remaining = []; + for (const cls of classValue.split(/\s+/)) { + if (!cls || cls === "bwi" || cls === iconName) continue; + if (cls === "bwi-fw") { + hasFixedWidth = true; + } else { + remaining.push(cls); + } + } + return { hasFixedWidth, remainingClasses: remaining.join(" ") }; +} export default { meta: { type: "suggestion", + hasSuggestions: true, docs: { description: "Discourage using 'bwi' font icon classes directly in favor of the component", category: "Best Practices", recommended: true, }, + messages: { + useBitIcon: errorMessage, + replaceBwi: "Replace with . Note: ensure IconModule is imported in your component.", + }, schema: [], }, create(context) { + /** + * Creates a fixer function if the element can be safely auto-fixed. + * Only fixes elements with static class attributes and a single extractable icon name. + */ + function createFix(node, classAttr, classValue) { + // Only auto-fix elements + if (node.name !== "i") { + return null; + } + + // Only fix static class attributes (not [class] or [ngClass] bindings) + const isStaticAttr = (node.attributes || []).includes(classAttr); + if (!isStaticAttr) { + return null; + } + + // Extract the icon name -- bail if none or ambiguous + const iconName = extractIconNameFromClassValue(classValue); + if (!iconName) { + return null; + } + + // Don't fix if the element has Angular bindings, outputs, or references + if ( + (node.inputs || []).length > 0 || + (node.outputs || []).length > 0 || + (node.references || []).length > 0 + ) { + return null; + } + + // Don't fix if the element has non-whitespace children (element nodes or non-empty text) + const hasContent = (node.children || []).some( + (child) => child.value === undefined || child.value.trim() !== "", + ); + if (hasContent) { + return null; + } + + // Get remaining classes (helpers + non-bwi classes) + const { hasFixedWidth, remainingClasses } = parseClasses(classValue, iconName); + + // Collect other attributes to preserve + // Drop: class (rebuilt above), aria-hidden="true" (bit-icon handles it automatically) + const otherAttrs = (node.attributes || []).filter((attr) => { + if (attr.name === "class") return false; + if (attr.name === "aria-hidden" && attr.value === "true") return false; + return true; + }); + + // Build the replacement element + const attrs = [`name="${iconName}"`]; + if (hasFixedWidth) { + attrs.push("fixedWidth"); + } + if (remainingClasses) { + attrs.push(`class="${remainingClasses}"`); + } + for (const attr of otherAttrs) { + attrs.push(attr.value != null ? `${attr.name}="${attr.value}"` : attr.name); + } + const replacement = ``; + + const start = node.sourceSpan.start.offset; + const end = node.sourceSpan.end.offset; + + return (fixer) => fixer.replaceTextRange([start, end], replacement); + } + return { Element(node) { // Get all class-related attributes @@ -45,21 +125,22 @@ export default { } // Extract all bwi classes from the class string - const bwiClassMatches = classValue.match(/\bbwi(?:-[\w-]+)?\b/g); + const bwiClassMatches = classValue.match(BWI_CLASS_RE); if (!bwiClassMatches) { continue; } // Check if any bwi class is NOT in the allowed helper classes list - const hasDisallowedBwiClass = bwiClassMatches.some( - (cls) => !ALLOWED_BWI_HELPER_CLASSES.has(cls), - ); + const hasDisallowedBwiClass = bwiClassMatches.some((cls) => !BWI_HELPER_CLASSES.has(cls)); if (hasDisallowedBwiClass) { + const fix = createFix(node, classAttr, classValue); + context.report({ node, - message: errorMessage, + messageId: "useBitIcon", + ...(fix ? { suggest: [{ messageId: "replaceBwi", fix }] } : {}), }); // Only report once per element break; diff --git a/libs/eslint/components/no-bwi-class-usage.spec.mjs b/libs/eslint/components/no-bwi-class-usage.spec.mjs index 768081ac966..922868a5783 100644 --- a/libs/eslint/components/no-bwi-class-usage.spec.mjs +++ b/libs/eslint/components/no-bwi-class-usage.spec.mjs @@ -12,11 +12,11 @@ ruleTester.run("no-bwi-class-usage", rule.default, { valid: [ { name: "should allow bit-icon component usage", - code: ``, + code: ``, }, { name: "should allow bit-icon with bwi-fw helper class", - code: ``, + code: ``, }, { name: "should allow bit-icon with name attribute and bwi-fw helper class", @@ -53,28 +53,108 @@ ruleTester.run("no-bwi-class-usage", rule.default, { ], invalid: [ { - name: "should error on direct bwi class usage", + name: "should suggest replacing direct bwi class usage with bit-icon", code: ``, - errors: [{ message: errorMessage }], + errors: [ + { + message: errorMessage, + suggestions: [{ messageId: "replaceBwi", output: `` }], + }, + ], }, { - name: "should error on bwi class with other classes", + name: "should suggest fix preserving non-bwi classes", code: ``, - errors: [{ message: errorMessage }], + errors: [ + { + message: errorMessage, + suggestions: [ + { + messageId: "replaceBwi", + output: ``, + }, + ], + }, + ], }, { - name: "should error on single bwi-* icon class", + name: "should suggest fix for single bwi-* icon class without base bwi", code: ``, - errors: [{ message: errorMessage }], + errors: [ + { + message: errorMessage, + suggestions: [{ messageId: "replaceBwi", output: `` }], + }, + ], }, { - name: "should error on icon classes even with helper classes", + name: "should suggest fix converting bwi-fw to fixedWidth attribute", code: ``, + errors: [ + { + message: errorMessage, + suggestions: [ + { + messageId: "replaceBwi", + output: ``, + }, + ], + }, + ], + }, + { + name: "should not suggest fix for base bwi class alone (no icon name)", + code: ``, errors: [{ message: errorMessage }], }, { - name: "should error on base bwi class alone", - code: ``, + name: "should suggest fix dropping aria-hidden='true'", + code: ``, + errors: [ + { + message: errorMessage, + suggestions: [{ messageId: "replaceBwi", output: `` }], + }, + ], + }, + { + name: "should suggest fix preserving other attributes and dropping aria-hidden", + code: ``, + errors: [ + { + message: errorMessage, + suggestions: [ + { + messageId: "replaceBwi", + output: ``, + }, + ], + }, + ], + }, + { + name: "should suggest fix with fixedWidth and preserve tw utility classes", + code: ``, + errors: [ + { + message: errorMessage, + suggestions: [ + { + messageId: "replaceBwi", + output: ``, + }, + ], + }, + ], + }, + { + name: "should not suggest fix for non-i elements (div)", + code: `
`, + errors: [{ message: errorMessage }], + }, + { + name: "should not suggest fix for non-i elements (span)", + code: ``, errors: [{ message: errorMessage }], }, ], diff --git a/libs/eslint/components/no-icon-children-in-bit-button.mjs b/libs/eslint/components/no-icon-children-in-bit-button.mjs index 926093f2e44..8a7c9ebd0ef 100644 --- a/libs/eslint/components/no-icon-children-in-bit-button.mjs +++ b/libs/eslint/components/no-icon-children-in-bit-button.mjs @@ -1,20 +1,171 @@ +import { extractIconNameFromClassValue } from "./bwi-utils.mjs"; + export const errorMessage = 'Avoid placing icon elements ( or ) inside a bitButton or bitLink. ' + "Use the [startIcon] or [endIcon] inputs instead. " + 'Example: '; +/** + * Extract the icon name from a child element. + * Supports both and . + * Returns null if the icon name cannot be determined. + */ +function extractIconName(child) { + if (child.name === "bit-icon") { + const nameAttr = (child.attributes || []).find((a) => a.name === "name"); + return nameAttr && typeof nameAttr.value === "string" ? nameAttr.value : null; + } + + if (child.name === "i") { + const classAttr = (child.attributes || []).find((a) => a.name === "class"); + if (!classAttr || typeof classAttr.value !== "string") return null; + return extractIconNameFromClassValue(classAttr.value); + } + + return null; +} + +// Classes that can safely be dropped when converting to startIcon/endIcon +// because the button component handles the equivalent spacing internally. +const DROPPABLE_CLASSES = new Set(["tw-mr-2", "tw-ml-2", "tw-ms-2", "tw-me-2"]); + +/** + * Check whether the icon child is simple enough for a clean suggestion. + * Returns false if the child has extra classes, attributes, or bindings + * that would be lost when converting to startIcon/endIcon. + */ +function isSimpleIconChild(child) { + if ((child.inputs || []).length > 0) return false; + + const hasContent = (child.children || []).some( + (c) => c.value === undefined || (typeof c.value === "string" && c.value.trim() !== ""), + ); + if (hasContent) return false; + + if (child.name === "i") { + const otherAttrs = (child.attributes || []).filter( + (a) => a.name !== "class" && !(a.name === "aria-hidden" && a.value === "true"), + ); + if (otherAttrs.length > 0) return false; + + const classAttr = (child.attributes || []).find((a) => a.name === "class"); + if (!classAttr) return false; + const classes = (classAttr.value || "").split(/\s+/).filter(Boolean); + return !classes.some((cls) => !cls.startsWith("bwi") && !DROPPABLE_CLASSES.has(cls)); + } + + if (child.name === "bit-icon") { + const otherAttrs = (child.attributes || []).filter((a) => { + if (a.name === "name") return false; + if (a.name === "aria-hidden" && a.value === "true") return false; + if (a.name === "class") { + const classes = (a.value || "").split(/\s+/).filter(Boolean); + return classes.some((cls) => !DROPPABLE_CLASSES.has(cls)); + } + return true; + }); + return otherAttrs.length === 0; + } + + return false; +} + +/** + * Determine whether the icon child is at the start or end of the parent's content. + * Returns "start", "end", or null (ambiguous). + */ +function getIconPosition(parent, iconChild) { + const children = parent.children || []; + const meaningfulChildren = children.filter( + (child) => child.name || (typeof child.value === "string" && child.value.trim() !== ""), + ); + + if (meaningfulChildren.length === 0) return "start"; + + const index = meaningfulChildren.indexOf(iconChild); + if (index === 0) return "start"; + if (index === meaningfulChildren.length - 1) return "end"; + return null; +} + export default { meta: { type: "suggestion", + hasSuggestions: true, docs: { description: "Discourage using icon child elements inside bitButton; use startIcon/endIcon inputs instead", category: "Best Practices", recommended: true, }, + messages: { + noIconChildren: errorMessage, + useStartIcon: "Replace with startIcon input on the parent element.", + useEndIcon: "Replace with endIcon input on the parent element.", + }, schema: [], }, create(context) { + /** + * Creates suggestions for replacing an icon child with startIcon/endIcon. + * Returns an empty array if the icon cannot be safely converted. + */ + function createSuggestions(parent, iconChild) { + const iconName = extractIconName(iconChild); + if (!iconName) return []; + + if (!isSimpleIconChild(iconChild)) return []; + + const parentAttrNames = [ + ...(parent.attributes?.map((a) => a.name) ?? []), + ...(parent.inputs?.map((i) => i.name) ?? []), + ]; + + const position = getIconPosition(parent, iconChild); + + const sourceCode = context.sourceCode ?? context.getSourceCode(); + const text = sourceCode.getText(); + + const makeFix = (inputName) => (fixer) => { + const insertOffset = parent.startSourceSpan.end.offset - 1; + + const { + start: { offset: removeStart }, + end: { offset: removeEnd }, + } = iconChild.sourceSpan; + + return [ + fixer.insertTextBeforeRange([insertOffset, insertOffset], ` ${inputName}="${iconName}"`), + fixer.removeRange([ + inputName === "startIcon" + ? removeStart + : removeStart - text.slice(0, removeStart).match(/ *$/)[0].length, + inputName === "startIcon" + ? removeEnd + text.slice(removeEnd).match(/^ */)[0].length + : removeEnd, + ]), + ]; + }; + + const suggestions = []; + + if ((position === "start" || position === null) && !parentAttrNames.includes("startIcon")) { + suggestions.push({ + messageId: "useStartIcon", + fix: makeFix("startIcon"), + }); + } + + if ((position === "end" || position === null) && !parentAttrNames.includes("endIcon")) { + suggestions.push({ + messageId: "useEndIcon", + fix: makeFix("endIcon"), + }); + } + + return suggestions; + } + return { Element(node) { if (node.name !== "button" && node.name !== "a") { @@ -37,9 +188,11 @@ export default { // child if (child.name === "bit-icon") { + const suggest = createSuggestions(node, child); context.report({ node: child, - message: errorMessage, + messageId: "noIconChildren", + ...(suggest.length ? { suggest } : {}), }); continue; } @@ -59,9 +212,11 @@ export default { } if (/\bbwi\b/.test(classValue)) { + const suggest = createSuggestions(node, child); context.report({ node: child, - message: errorMessage, + messageId: "noIconChildren", + ...(suggest.length ? { suggest } : {}), }); break; } diff --git a/libs/eslint/components/no-icon-children-in-bit-button.spec.mjs b/libs/eslint/components/no-icon-children-in-bit-button.spec.mjs index 656a320678d..7fb67e9f2cc 100644 --- a/libs/eslint/components/no-icon-children-in-bit-button.spec.mjs +++ b/libs/eslint/components/no-icon-children-in-bit-button.spec.mjs @@ -1,6 +1,6 @@ import { RuleTester } from "@typescript-eslint/rule-tester"; -import rule, { errorMessage } from "./no-icon-children-in-bit-button.mjs"; +import rule from "./no-icon-children-in-bit-button.mjs"; const ruleTester = new RuleTester({ languageOptions: { @@ -49,49 +49,192 @@ ruleTester.run("no-icon-children-in-bit-button", rule.default, { ], invalid: [ { - name: "should warn on with bwi class inside button[bitButton]", + name: "should suggest startIcon for with bwi class inside button[bitButton]", code: ``, - errors: [{ message: errorMessage }], + errors: [ + { + messageId: "noIconChildren", + suggestions: [ + { + messageId: "useStartIcon", + output: ``, + }, + ], + }, + ], }, { - name: "should warn on with bwi class and extra classes inside button[bitButton]", + name: "should suggest fix when has droppable spacing classes", code: ``, - errors: [{ message: errorMessage }], + errors: [ + { + messageId: "noIconChildren", + suggestions: [ + { + messageId: "useStartIcon", + output: ``, + }, + ], + }, + ], }, { - name: "should warn on with bwi class inside a[bitButton]", + name: "should not suggest fix when has non-droppable classes", + code: ``, + errors: [{ messageId: "noIconChildren" }], + }, + { + name: "should suggest startIcon for with bwi class inside a[bitButton]", code: ` Link`, - errors: [{ message: errorMessage }], + errors: [ + { + messageId: "noIconChildren", + suggestions: [ + { + messageId: "useStartIcon", + output: `Link`, + }, + ], + }, + ], }, { - name: "should warn on inside button[bitButton]", + name: "should suggest startIcon for inside button[bitButton]", code: ``, - errors: [{ message: errorMessage }], + errors: [ + { + messageId: "noIconChildren", + suggestions: [ + { + messageId: "useStartIcon", + output: ``, + }, + ], + }, + ], }, { - name: "should warn on inside a[bitButton]", + name: "should suggest startIcon for inside a[bitButton]", code: ` Copy`, - errors: [{ message: errorMessage }], + errors: [ + { + messageId: "noIconChildren", + suggestions: [ + { + messageId: "useStartIcon", + output: `Copy`, + }, + ], + }, + ], }, { - name: "should warn on multiple icon children inside bitButton", + name: "should suggest startIcon and endIcon for multiple icon children", code: ``, - errors: [{ message: errorMessage }, { message: errorMessage }], + errors: [ + { + messageId: "noIconChildren", + suggestions: [ + { + messageId: "useStartIcon", + output: ``, + }, + ], + }, + { + messageId: "noIconChildren", + suggestions: [ + { + messageId: "useEndIcon", + output: ``, + }, + ], + }, + ], }, { - name: "should warn on both and children", + name: "should suggest for both and children", code: ``, - errors: [{ message: errorMessage }, { message: errorMessage }], + errors: [ + { + messageId: "noIconChildren", + suggestions: [ + { + messageId: "useStartIcon", + output: ``, + }, + ], + }, + { + messageId: "noIconChildren", + suggestions: [ + { + messageId: "useEndIcon", + output: ``, + }, + ], + }, + ], }, { - name: "should warn on with bwi class inside a[bitLink]", + name: "should suggest startIcon for with bwi class inside a[bitLink]", code: ` Link`, - errors: [{ message: errorMessage }], + errors: [ + { + messageId: "noIconChildren", + suggestions: [ + { + messageId: "useStartIcon", + output: `Link`, + }, + ], + }, + ], }, { - name: "should warn on inside button[bitLink]", + name: "should suggest startIcon for inside button[bitLink]", code: ``, - errors: [{ message: errorMessage }], + errors: [ + { + messageId: "noIconChildren", + suggestions: [ + { + messageId: "useStartIcon", + output: ``, + }, + ], + }, + ], + }, + { + name: "should suggest endIcon when icon is after text", + code: ``, + errors: [ + { + messageId: "noIconChildren", + suggestions: [ + { + messageId: "useEndIcon", + output: ``, + }, + ], + }, + ], + }, + { + name: "should suggest startIcon for with aria-hidden (no extra classes)", + code: ``, + errors: [ + { + messageId: "noIconChildren", + suggestions: [ + { + messageId: "useStartIcon", + output: ``, + }, + ], + }, + ], }, ], });