From 6577b101d2dd804cd0bd5504a0b597e53d43a73f Mon Sep 17 00:00:00 2001 From: SimeonC <1085899+SimeonC@users.noreply.github.com> Date: Wed, 27 Nov 2024 10:32:01 +0900 Subject: [PATCH] fix: correctly scope checkboxes and radios in classy to avoid conflicts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removed `width` calculation that was ignored as `calc(var(…)+ 2)` is ignored due to `var(…)+` being invalid. A space is required between `)` and `+` --- system/core/scripts/generateComponents.ts | 99 +++++++++++-------- system/core/src/components/Checkbox.ts | 1 + .../core/src/components/CheckboxRadioLabel.ts | 2 + system/core/src/components/InputLikeButton.ts | 1 + system/core/src/components/InputWithIcons.ts | 4 - system/core/src/components/InputWithPrefix.ts | 4 - system/core/src/components/Radio.ts | 1 + system/core/src/components/Spinner.ts | 1 + .../core/src/components/TextAreaWithIcons.ts | 4 - .../core/src/components/TextAreaWithPrefix.ts | 4 - system/core/src/components/Tooltip.ts | 1 + system/css/src/buildFile.ts | 16 ++- system/react-css/src/components/Badge.tsx | 6 +- system/react-css/src/components/Banner.tsx | 6 +- system/react-css/src/components/Button.tsx | 8 +- system/react-css/src/components/Checkbox.tsx | 2 +- .../react-css/src/components/IconButton.tsx | 8 +- system/react-css/src/components/Radio.tsx | 2 +- system/react-css/src/utils.tsx | 18 ++-- 19 files changed, 111 insertions(+), 77 deletions(-) diff --git a/system/core/scripts/generateComponents.ts b/system/core/scripts/generateComponents.ts index ae4587a26..b95afc0bf 100644 --- 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,8 +70,12 @@ 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 @@ -79,15 +83,15 @@ abstract class ComponentBuilder { } 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' }); @@ -249,9 +253,13 @@ 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; } @@ -259,10 +267,8 @@ class CssComponentBuilder extends ComponentBuilder { } 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 +283,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 +296,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 +313,37 @@ class CssComponentBuilder extends ComponentBuilder { writeForwardRefComponent({ varName, elementName, - omitVariants = false, displayName = '', + omitVariants = false, fixedProps: fixedPropsArg = this.fixedProps || [], shouldExport = false }) { - const propsType = omitVariants ? 'Omit' : 'Props'; + const propsType = `${ + omitVariants ? 'Omit' : '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 +355,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 +420,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 index c8ef1f6fb..a7ef5856a 100644 --- 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 index 136b25d73..38153df1c 100644 --- 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 index f0ee5b224..642582863 100644 --- 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 index 4a2001664..148778803 100644 --- 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 index 96230f56c..198a88833 100644 --- 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 index 9e2ef7d71..d5b989b9a 100644 --- 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 index fa87c5043..319435f0b 100644 --- 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/TextAreaWithIcons.ts b/system/core/src/components/TextAreaWithIcons.ts index 0a0515dbe..8d0d2256e 100644 --- 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 index b67239761..872a5c461 100644 --- 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 index 62b8601be..92756f970 100644 --- 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 index b07776dcc..adc08732e 100644 --- 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; @@ -139,8 +140,19 @@ export class ComponentBuilder extends CssFileBuilder { } 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 index ced8ca868..43596350c 100644 --- 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 index 4ede4f6fe..a7e77f01d 100644 --- 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 index a917d01f6..6d88899a8 100644 --- 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 index 8b58ad399..c89bd5022 100644 --- 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 } }); diff --git a/system/react-css/src/components/IconButton.tsx b/system/react-css/src/components/IconButton.tsx index e031bd0a0..34009c9b2 100644 --- a/system/react-css/src/components/IconButton.tsx +++ b/system/react-css/src/components/IconButton.tsx @@ -46,7 +46,11 @@ export const VariantIconButton = buildVariantComponents< 'danger', 'danger-bare' ], + tag: 'button', + displayName: 'IconButton', className: 'icon-button', - element: 'button', - displayName: 'IconButton' + additionalProps: { + type: 'button', + 'data-size': { toString: () => getConfigDefault('controlSize') } + } }); diff --git a/system/react-css/src/components/Radio.tsx b/system/react-css/src/components/Radio.tsx index 1daab1055..9c2dfd4d6 100644 --- a/system/react-css/src/components/Radio.tsx +++ b/system/react-css/src/components/Radio.tsx @@ -16,6 +16,6 @@ export const Radio = buildWithComponent< >({ tag: 'input', displayName: 'Radio', - className: undefined, + className: 'radio', additionalProps: { type: radio.defaultProps.type as never } }); diff --git a/system/react-css/src/utils.tsx b/system/react-css/src/utils.tsx index 29e89f932..d6b6a2ad5 100644 --- a/system/react-css/src/utils.tsx +++ b/system/react-css/src/utils.tsx @@ -140,14 +140,13 @@ export function buildVariantComponents< >({ variants, className, - element, - displayName + tag, + displayName, + additionalProps, + style }: { variants: TVariant[]; - className: string; - displayName: string; - element: TElement; -}): Record< +} & DecorateOptions): Record< Capitalize, React.ForwardRefExoticComponent & WithComponentType > { @@ -161,10 +160,13 @@ export function buildVariantComponents< HTMLElementTagNameMap[TElement], Omit >({ - tag: element, + tag, className, + style, displayName: `${displayName}_${variantKey}`, - additionalProps: { 'data-variant': key } + additionalProps: additionalProps + ? { ...additionalProps, 'data-variant': key } + : { 'data-variant': key } }) }; }, {} as Record, React.ForwardRefExoticComponent & WithComponentType>);