fix: correctly scope checkboxes and radios in classy to avoid conflicts

Removed `width` calculation that was ignored as `calc(var(…)+ 2)` is ignored due to `var(…)+` being invalid. A space is required between `)` and `+`
tablecheck · Nov 28, 2024 · 38e9303 · 38e9303
1 parent 75dc858
commit 38e9303
Show file tree

Hide file tree

Showing 24 changed files with 136 additions and 99 deletions.
diff --git a/system/core/scripts/generateComponents.ts b/system/core/scripts/generateComponents.ts
@@ -29,7 +29,7 @@ abstract class ComponentBuilder {
 
   protected importKey: string;
 
-  protected element: string;
+  protected element: string | undefined;
 
   protected filePath: string;
 
@@ -70,24 +70,28 @@ abstract class ComponentBuilder {
     return `HTML${this.reactElementType}Element`;
   }
 
+  get safeElement() {
+    return this.element || 'div';
+  }
+
   get reactHtmlAttributesType() {
-    if (['span', 'div'].includes(this.element))
+    if (['span', 'div'].includes(this.safeElement))
       return `React.HTMLAttributes<${this.elementType}>`;
     return `React.${
       this.reactElementType === 'TextArea' ? 'Textarea' : this.reactElementType
     }HTMLAttributes<${this.elementType}>`;
   }
 
   get reactElementType() {
-    if (this.element === 'a') return 'Anchor';
-    if (this.element === 'textarea') return 'TextArea';
-    return capitalize(this.element);
+    if (this.safeElement === 'a') return 'Anchor';
+    if (this.safeElement === 'textarea') return 'TextArea';
+    return capitalize(this.safeElement);
   }
 
   constructor(outputFolderPath, importedKey, importedValues) {
     this.fixedProps = [];
     this.importKey = importedKey;
-    this.element = importedValues.element || 'div';
+    this.element = importedValues.element;
     this.variantStyles = importedValues.variantStyles;
     const defaultProps = importedValues.defaultProps
       ? { ...importedValues.defaultProps }
@@ -165,7 +169,7 @@ abstract class ComponentBuilder {
     elementName,
     omitVariants,
     displayName,
-    fixedProps = this.fixedProps || []
+    fixedProps
   }): string;
 
   writeForwardRefComponentFunction({
@@ -193,7 +197,7 @@ abstract class ComponentBuilder {
       });
     });
     if (
-      this.element === 'button' &&
+      this.safeElement === 'button' &&
       !this.defaultProps.find(([key]) => key === 'type')
     ) {
       props.push({ key: 'type', type: 'string', value: 'button' });
@@ -233,10 +237,7 @@ class CssComponentBuilder extends ComponentBuilder {
 
   constructor(importedKey, importedValues) {
     super(cssOutputFolderPath, importedKey, importedValues);
-    this.className =
-      this.pascalImportKey === 'InputLikeButton'
-        ? 'input'
-        : importedValues.className || '';
+    this.className = importedValues.className || '';
     this.fixedProps = this.className
       ? [
           {
@@ -249,20 +250,22 @@ class CssComponentBuilder extends ComponentBuilder {
   }
 
   isValidComponentImport() {
-    if (!this.hasValidSelectors()) {
+    const isValid = this.hasValidSelectors();
+    if (!isValid) {
+      throw new Error(`${this.importKey} does not have valid selectors`);
+    }
+    if (isValid === 'skip') {
       console.warn(
-        `Skipping ${this.importKey} as it does not have valid selectors`
+        `Skipping ${this.importKey} as it does not export 'element' or className`
       );
       return false;
     }
     return super.isValidComponentImport();
   }
 
   hasValidSelectors() {
-    if (this.pascalImportKey === 'Spinner') return true;
-    if (this.className) return true;
-    if (!this.element) return false;
-    return !!this.defaultProps.find(([key]) => key === 'type');
+    if (!this.element && !this.className) return 'skip';
+    return !!this.className;
   }
 
   buildFileContent() {
@@ -277,7 +280,7 @@ class CssComponentBuilder extends ComponentBuilder {
     fileContent.push(
       this.writeForwardRefComponent({
         varName: this.pascalImportKey,
-        elementName: this.element,
+        elementName: this.safeElement,
         displayName: this.pascalImportKey,
         shouldExport: true
       })
@@ -290,12 +293,13 @@ class CssComponentBuilder extends ComponentBuilder {
               .map((v) => `'${v}'`)
               .join(' | ')},
             Props,
-            '${this.element}'
+            '${this.safeElement}'
         >({
           variants: ${JSON.stringify(this.getVariants())},
-          className: '${this.className}',
-          element: '${this.element}',
-          displayName: '${this.pascalImportKey}'
+          ${this.buildWithComponentProps({
+            elementName: this.safeElement,
+            displayName: this.pascalImportKey
+          })},
         });`
       );
     }
@@ -306,17 +310,37 @@ class CssComponentBuilder extends ComponentBuilder {
   writeForwardRefComponent({
     varName,
     elementName,
-    omitVariants = false,
     displayName = '',
+    omitVariants = false,
     fixedProps: fixedPropsArg = this.fixedProps || [],
     shouldExport = false
   }) {
-    const propsType = omitVariants ? 'Omit<Props, "data-variant">' : 'Props';
+    const propsType = `${
+      omitVariants ? 'Omit<Props, "data-variant">' : 'Props'
+    } & ${this.reactHtmlAttributesType}`;
+    return `${
+      shouldExport ? 'export ' : ''
+    }const ${varName} = buildWithComponent<${
+      this.elementType
+    }, ${propsType}>({ ${this.buildWithComponentProps({
+      elementName,
+      displayName: displayName || varName,
+      fixedProps: fixedPropsArg
+    })}});`;
+  }
+
+  // eslint-disable-next-line @typescript-eslint/naming-convention
+  private buildWithComponentProps({
+    elementName,
+    displayName = '',
+    fixedProps: fixedPropsArg = this.fixedProps || []
+  }) {
     const props = this.buildProps(fixedPropsArg);
     const classNameProp = props.find(({ key }) => key === 'className');
-    const append: string[] = [
-      `className: ${classNameProp ? `'${classNameProp.value}'` : 'undefined'}`
-    ];
+    if (!classNameProp) {
+      throw new Error(`Missing className prop for ${displayName}`);
+    }
+    const append: string[] = [`className: '${classNameProp.value}'`];
     const additionalProps = props.filter(({ key }) => key !== 'className');
     if (additionalProps.length) {
       append.push(
@@ -328,15 +352,9 @@ class CssComponentBuilder extends ComponentBuilder {
           .join(', ')} }`
       );
     }
-    return `${
-      shouldExport ? 'export ' : ''
-    }const ${varName} = buildWithComponent<${
-      this.elementType
-    }, ${propsType} & ${
-      this.reactHtmlAttributesType
-    }>({ tag: '${elementName}', displayName: '${
-      displayName || varName
-    }', ${append.join(', ')}});`;
+    return `tag: '${elementName}', displayName: '${displayName}', ${append.join(
+      ', '
+    )}`;
   }
 
   protected formatDefaultProp(key: string) {
@@ -399,15 +417,15 @@ class ReactComponentBuilder extends ComponentBuilder {
     }
     fileContent.push('');
     fileContent.push(
-      `const Base = styled.${this.element}<${this.getBasePropsType()}>\`\${${
-        this.importKey
-      }.fullStyles}\`;`
+      `const Base = styled.${
+        this.safeElement
+      }<${this.getBasePropsType()}>\`\${${this.importKey}.fullStyles}\`;`
     );
     if (this.coreStyles) {
       fileContent.push(
-        `const Core = styled.${this.element}<${this.getBasePropsType()}>\`\${${
-          this.importKey
-        }.coreStyles}\`;`
+        `const Core = styled.${
+          this.safeElement
+        }<${this.getBasePropsType()}>\`\${${this.importKey}.coreStyles}\`;`
       );
     }
 

diff --git a/system/core/src/components/Checkbox.ts b/system/core/src/components/Checkbox.ts
@@ -2,6 +2,7 @@ import { OptionalKeys, css } from '../utils';
 
 export const element = 'input';
 export const selectors = ['input[type="checkbox"]:not(.toggle)'];
+export const className = 'checkbox';
 
 export interface Props {
   type: 'checkbox';

diff --git a/system/core/src/components/CheckboxRadioLabel.ts b/system/core/src/components/CheckboxRadioLabel.ts
@@ -2,6 +2,8 @@ import { css } from '../utils';
 
 export const element = 'label';
 export const selectors = ['label.checkbox', 'label.radio'];
+export const className = 'radio';
+export const classNameSelector = 'label.checkbox, label.radio';
 
 export interface Props {
   htmlFor?: string;

diff --git a/system/core/src/components/InputLikeButton.ts b/system/core/src/components/InputLikeButton.ts
@@ -10,6 +10,7 @@ export type {
 
 export const element = 'button';
 export const selectors = ['button.input', 'a.input'];
+export const className = 'input-like-button';
 
 export const defaultProps = {
   role: 'button'

diff --git a/system/core/src/components/InputWithIcons.ts b/system/core/src/components/InputWithIcons.ts
@@ -77,10 +77,6 @@ export const fullStyles = css`
   }
   & > [data-mode='input-append'] {
     height: calc(var(--tk-input-height) - var(--tk-input-border-width) * 2);
-    width: calc(
-      var(--tk-input-icon-size)+ var(--tk-input-icon-gap)+
-        var(--tk-input-icon-end-padding)
-    );
     --tk-icon-button-padding: 8px !important;
     margin-top: 0;
     margin-bottom: 0;

diff --git a/system/core/src/components/InputWithPrefix.ts b/system/core/src/components/InputWithPrefix.ts
@@ -85,10 +85,6 @@ export const fullStyles = css`
   & > [data-mode='input-append'] {
     grid-area: 1/4/1/5;
     height: calc(var(--tk-input-height) - var(--tk-input-border-width) * 2);
-    width: calc(
-      var(--tk-input-icon-size)+ var(--tk-input-icon-gap)+
-        var(--tk-input-icon-end-padding)
-    );
     --tk-icon-button-padding: 8px !important;
     margin-top: 0;
     margin-bottom: 0;

diff --git a/system/core/src/components/Radio.ts b/system/core/src/components/Radio.ts
@@ -2,6 +2,7 @@ import { OptionalKeys, css } from '../utils';
 
 export const element = 'input';
 export const selectors = ['input[type="radio"]:not(.toggle)'];
+export const className = 'radio';
 
 export interface Props {
   type: 'radio';

diff --git a/system/core/src/components/Spinner.ts b/system/core/src/components/Spinner.ts
@@ -8,6 +8,7 @@ export const element = 'span';
 export const selectors = [
   '[aria-busy="true"]:not(button):not(select):not(input):not(textarea)'
 ];
+export const className = 'spinner';
 
 export interface Props {
   'aria-busy': boolean;

diff --git a/system/core/src/components/Table.ts b/system/core/src/components/Table.ts
@@ -42,10 +42,16 @@ export const fullStyles = css`
 
     & tr:hover {
       --tk-table-row-background: var(--surface-raised-hover);
+      [data-variant='bare']:hover {
+        --tk-button-background-color: var(--surface-hover-transparent);
+      }
     }
 
     & tr[data-active='true'] {
       --tk-table-row-background: var(--surface-raised-active);
+      [data-variant='bare']:hover {
+        --tk-button-background-color: var(--surface-hover-transparent);
+      }
     }
   }
 

diff --git a/system/core/src/components/TextAreaWithIcons.ts b/system/core/src/components/TextAreaWithIcons.ts
@@ -96,10 +96,6 @@ export const fullStyles = css`
   }
   & > [data-mode='input-append'] {
     height: calc(var(--tk-input-height) - var(--tk-input-border-width) * 2);
-    width: calc(
-      var(--tk-input-icon-size)+ var(--tk-input-icon-gap)+
-        var(--tk-input-icon-end-padding)
-    );
     --tk-icon-button-padding: 8px !important;
     margin-top: 0;
     margin-bottom: 0;

diff --git a/system/core/src/components/TextAreaWithPrefix.ts b/system/core/src/components/TextAreaWithPrefix.ts
@@ -101,10 +101,6 @@ export const fullStyles = css`
   & > [data-mode='input-append'] {
     grid-area: 1/4/1/5;
     height: calc(var(--tk-input-height) - var(--tk-input-border-width) * 2);
-    width: calc(
-      var(--tk-input-icon-size)+ var(--tk-input-icon-gap)+
-        var(--tk-input-icon-end-padding)
-    );
     --tk-icon-button-padding: 8px !important;
     margin-top: 0;
     margin-bottom: 0;

diff --git a/system/core/src/components/Tooltip.ts b/system/core/src/components/Tooltip.ts
@@ -6,6 +6,7 @@ import { css } from '../utils';
 
 export const element = 'span';
 export const selectors = ['[data-tooltip]'];
+export const className = 'tooltip';
 
 export interface Props {
   'data-tooltip': string;

diff --git a/system/css/src/buildFile.ts b/system/css/src/buildFile.ts
@@ -118,6 +118,7 @@ export class ThemeVariablesBuilder extends CssFileBuilder {
 interface ComponentFileContent {
   element?: string;
   className?: string;
+  classNameSelector?: string;
   selectors?: string[];
   fullStyles: string;
   variantStyles?: Record<string, string>;
@@ -139,8 +140,19 @@ export class ComponentBuilder extends CssFileBuilder<ComponentFileContent> {
   }
 
   getClassySelectors() {
-    const { element, className, selectors = [] } = this.fileContent;
-    if (!className) return selectors;
+    const {
+      element,
+      className,
+      classNameSelector,
+      selectors = []
+    } = this.fileContent;
+    if (!className) {
+      throw new Error(
+        `className is missing in ${this.folderName}/${this.fileName}`
+      );
+      return selectors;
+    }
+    if (classNameSelector) return [classNameSelector];
     if (element === 'input') return [`input.${className}`];
     return [`.${className}`];
   }

diff --git a/system/react-css/src/components/Badge.tsx b/system/react-css/src/components/Badge.tsx
@@ -40,7 +40,7 @@ export const VariantBadge = buildVariantComponents<
     'orange',
     'disabled'
   ],
-  className: 'badge',
-  element: 'span',
-  displayName: 'Badge'
+  tag: 'span',
+  displayName: 'Badge',
+  className: 'badge'
 });
diff --git a/system/react-css/src/components/Banner.tsx b/system/react-css/src/components/Banner.tsx
@@ -20,7 +20,7 @@ export const VariantBanner = buildVariantComponents<
   'div'
 >({
   variants: ['tertiary', 'ghost', 'success', 'warning', 'info', 'neutral'],
-  className: 'banner',
-  element: 'div',
-  displayName: 'Banner'
+  tag: 'div',
+  displayName: 'Banner',
+  className: 'banner'
 });
diff --git a/system/react-css/src/components/Button.tsx b/system/react-css/src/components/Button.tsx
@@ -44,7 +44,11 @@ export const VariantButton = buildVariantComponents<
     'bare',
     'danger'
   ],
+  tag: 'button',
+  displayName: 'Button',
   className: 'btn',
-  element: 'button',
-  displayName: 'Button'
+  additionalProps: {
+    type: button.defaultProps.type as never,
+    'data-size': { toString: () => getConfigDefault('controlSize') }
+  }
 });
diff --git a/system/react-css/src/components/Checkbox.tsx b/system/react-css/src/components/Checkbox.tsx
@@ -16,6 +16,6 @@ export const Checkbox = buildWithComponent<
 >({
   tag: 'input',
   displayName: 'Checkbox',
-  className: undefined,
+  className: 'checkbox',
   additionalProps: { type: checkbox.defaultProps.type as never }
 });