🚨 Fix some lint warning #240
Conversation
WalkthroughThis pull request encompasses a series of minor modifications across multiple React components in the packages directory. The changes primarily involve formatting adjustments, such as switching quotation marks from double to single, removing unused imports, and making subtle type refinements. These modifications aim to improve code consistency, remove unnecessary code, and enhance type safety without significantly altering the core functionality of the components. Changes
Possibly related PRs
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (13)
packages/react/components/columns/Columns.native.tsx (1)
91-91: Consider using proper React types instead of 'any'While the eslint-disable comment fixes the immediate lint warning, using
anytype bypasses TypeScript's type checking benefits. Consider using proper React types for better type safety:- // eslint-disable-next-line @typescript-eslint/no-explicit-any - (child: any) => + (child: React.ReactElement) =>This would provide better type safety while still resolving the lint warning.
packages/react/components/radio/Radio.native.tsx (2)
Line range hint
59-71: Good type refinement, consider adding value validation.The change from
anytostringtype improves type safety and matches the component's props interface.Consider adding validation for empty string values:
- const handleClick = (value: string) => { + const handleClick = (value: string) => { + if (!value.trim()) { + return; + } if (!readonly) { if (onChange) { onChange({ radioId: id, radioValue: value, radioName: name || '', radioChecked: true, }) } } }
73-73: Good operator change, consider memoizing the handler.The switch to nullish coalescing (
??) is more precise than logical OR for handling undefined/null values.Consider memoizing the onPress handler to prevent unnecessary re-renders:
+ const onPressHandler = React.useCallback(() => handleClick(value ?? ''), [value, handleClick]); - <TouchableOpacity disabled={disabled} style={styles.container} onPress={() => handleClick(value ?? '')}> + <TouchableOpacity disabled={disabled} style={styles.container} onPress={onPressHandler}>packages/react/components/select/Select.native.tsx (1)
61-63: Simplify the type checking logicThe current implementation has complex type checking that could be simplified for better readability and maintainability.
Consider this cleaner implementation:
- (multiple && selectedValues && typeof selectedValues !== 'string' && typeof selectedValues !== 'number' - ? selectedValues?.includes(value) - : selectedValues === value), + multiple && Array.isArray(selectedValues) + ? selectedValues?.includes(value) + : selectedValues === value,This change:
- Uses
Array.isArray()instead of multiple type checks- Maintains the same functionality
- Is more readable and less prone to lint warnings
packages/react/components/select/web/SelectDynamic.tsx (2)
177-177: Remove extra space in role attributeThere's an extra space between
role='listbox'and the className prop.- {focused && <ul role='listbox' className={hashClass(styled, clsx('select-options'))}>{options}</ul>} + {focused && <ul role='listbox' className={hashClass(styled, clsx('select-options'))}>{options}</ul>}
2-7: Consider extracting shared logic into custom hooksBoth
Select.native.tsxandSelectDynamic.tsxshare similar logic for handling selections and state management. Consider creating custom hooks to:
- Manage selection state
- Handle focus state
- Process options
This would:
- Reduce code duplication
- Improve maintainability
- Make the components more focused
Example structure:
// useSelect.ts export const useSelect = (props: SelectProps) => { const [selectedValues, setSelectedValues] = useState<SelectedValue>(props.selected); const [selectedNames, setSelectedNames] = useState<string[]>([]); // ... shared logic for isChecked, setNewSelectedValues, etc. return { selectedValues, selectedNames, isChecked, setNewSelectedValues, // ... other shared functionality }; };packages/react/components/input/Input.tsx (2)
Line range hint
301-302: Consider improving type safetyThe type assertion
as unknown as IconNameis a code smell that might indicate underlying type issues. Consider defining proper types foriconNameLeftandiconNameRightprops instead of using type assertions.- {iconNameLeft && !loading && <IconWrapper className={'icon-left'} name={iconNameLeft as unknown as IconName} />} + {iconNameLeft && !loading && <IconWrapper className={'icon-left'} name={iconNameLeft} />}This would require updating the
InputPropinterface to properly type these props:interface InputProp extends Accessibility, InputProps, InputWebEvents { iconNameLeft?: IconName; iconNameRight?: IconName; }
Line range hint
1-324: Consider consolidating state managementThe component uses multiple
useStatehooks that are closely related. Consider usinguseReducerto manage related state in a more maintainable way, particularly for:
isFocused,isDirty, andisTouchedinputTypeandisShowPwdpackages/react/components/tag/Tag.native.tsx (1)
Line range hint
8-18: Update JSDoc to match implementationThe JSDoc still documents the
iconNameparameter but according to the AI summary, this prop was intended to be removed.- * @param iconName {IconName} display iconpackages/react/components/price/Price.tsx (1)
Line range hint
147-150: Consider extracting accessibility text generationThe accessibility label logic could be extracted into a helper function for better maintainability.
+const getPriceAccessibilityLabel = (accessibilityLabel: string | undefined, priceText: string): string => { + return accessibilityLabel || priceText || 'NotSpecified' +} + const Price = ({ // ...props }) => { // ... const priceText = `${whole}€${hideCents ? cents : ''}${mention ? mention : ''}${period ? ` / ${period}` : ''}` const priceTestId = testId ? testId : priceText ? priceText : 'NotSpecified' - const priceAccessibilityLabel = accessibilityLabel ? accessibilityLabel : priceText ? priceText : 'NotSpecified' + const priceAccessibilityLabel = getPriceAccessibilityLabel(accessibilityLabel, priceText)packages/react/components/price/Price.native.tsx (2)
Line range hint
108-166: Consider extracting level-based style calculationsThe price level calculations are complex and repeated. Consider extracting them into helper functions for better maintainability.
+const getPriceLevelSize = (level: PriceLevel): number => { + switch(level) { + case PriceLevel.ONE: return 64; + case PriceLevel.TWO: return 56; + case PriceLevel.THREE: return 44; + case PriceLevel.FOUR: return 32; + case PriceLevel.FIVE: return 28; + case PriceLevel.SIX: return 24; + case PriceLevel.SEVEN: return 20; + default: return 44; + } +} const Price = ({ // ...props }) => { // ... - const priceLevel = - (level == PriceLevel.ONE && 64) || - (level == PriceLevel.TWO && 56) || - (level == PriceLevel.THREE && 44) || - (level == PriceLevel.FOUR && 32) || - (level == PriceLevel.FIVE && 28) || - (level == PriceLevel.SIX && 24) || - (level == PriceLevel.SEVEN && 20) || - 44 + const priceLevel = getPriceLevelSize(level)
Line range hint
168-219: Extract rotation and position calculationsThe
strikedRotateByLevelandstrikedBottomByLevelfunctions contain complex conditional logic that could be simplified using lookup tables or helper functions.+const ROTATION_MAP = { + [PriceLevel.SEVEN]: { withCents: '-18deg', default: '-22deg' }, + [PriceLevel.SIX]: { withCents: '-19deg', default: '-22deg' }, + // ... add other levels +}; +const getStrikedRotation = (level: PriceLevel, hasCents: boolean): string => { + if (!level) return hasCents ? '-15deg' : '-20deg'; + return ROTATION_MAP[level]?.[hasCents ? 'withCents' : 'default'] ?? '-20deg'; +} const Price = ({ // ...props }) => { // ... - const strikedRotateByLevel = () => { - return ( - (level == PriceLevel.SEVEN && (hideCents || period) && '-18deg') || - // ... existing conditions - '-20deg' - ) - } + const strikedRotation = getStrikedRotation(level, hideCents || !!period)packages/react/objects/facets/Color.tsx (1)
Line range hint
126-132: Consider improving browser detection and error handling.The current browser detection logic using
navigator.userAgent === undefinedseems unusual and might not reliably detect all environments.Consider:
- Using a more standard approach for environment detection
- Adding proper error handling for undefined color values
export const getColorStyle = (trilogyColor: TrilogyColor | TrilogyColorValues): string => { - if (typeof navigator !== 'undefined' && navigator.userAgent === undefined) { + if (typeof window !== 'undefined') { const { theme } = useContext(TrilogyThemeContext) const colorsStyle = theme?.colors || colors - const colorArray = colorsStyle[trilogyColor] || colorsStyle.default - if (!trilogyColor || !colors[trilogyColor]) { - return colorsStyle.default + if (!trilogyColor || !colorsStyle[trilogyColor]) { + return colorsStyle[TrilogyColor.MAIN][0] } - return colorArray[0] + return colorsStyle[trilogyColor][0] } else { return colors[trilogyColor][0] || colors[TrilogyColor.MAIN][0] } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
packages/react/components/badge/Badge.native.tsx(1 hunks)packages/react/components/breadcrumb/Breadcrumb.tsx(1 hunks)packages/react/components/chips/list/ChipsList.tsx(1 hunks)packages/react/components/columns/Columns.native.tsx(1 hunks)packages/react/components/divider/Divider.native.tsx(0 hunks)packages/react/components/icon/Icon.tsx(1 hunks)packages/react/components/icon/IconProps.ts(1 hunks)packages/react/components/input/Input.tsx(2 hunks)packages/react/components/link/Link.native.tsx(0 hunks)packages/react/components/modal/Modal.tsx(0 hunks)packages/react/components/price/Price.native.tsx(1 hunks)packages/react/components/price/Price.tsx(1 hunks)packages/react/components/progress/Progress.tsx(1 hunks)packages/react/components/progress/radial/ProgressRadial.tsx(1 hunks)packages/react/components/radio/Radio.native.tsx(2 hunks)packages/react/components/radio/list/RadioList.native.tsx(1 hunks)packages/react/components/select/Select.native.tsx(1 hunks)packages/react/components/select/option/SelectOption.tsx(2 hunks)packages/react/components/select/web/SelectDynamic.tsx(3 hunks)packages/react/components/stepper/Stepper.tsx(1 hunks)packages/react/components/tag/Tag.native.tsx(1 hunks)packages/react/components/title/Title.native.tsx(0 hunks)packages/react/objects/Typography/TypographyBold.tsx(0 hunks)packages/react/objects/Typography/TypographyColor.ts(1 hunks)packages/react/objects/facets/Color.tsx(2 hunks)
💤 Files with no reviewable changes (5)
- packages/react/components/link/Link.native.tsx
- packages/react/components/divider/Divider.native.tsx
- packages/react/components/title/Title.native.tsx
- packages/react/objects/Typography/TypographyBold.tsx
- packages/react/components/modal/Modal.tsx
✅ Files skipped from review due to trivial changes (9)
- packages/react/components/breadcrumb/Breadcrumb.tsx
- packages/react/components/chips/list/ChipsList.tsx
- packages/react/components/stepper/Stepper.tsx
- packages/react/components/select/option/SelectOption.tsx
- packages/react/components/badge/Badge.native.tsx
- packages/react/components/radio/list/RadioList.native.tsx
- packages/react/components/icon/Icon.tsx
- packages/react/objects/Typography/TypographyColor.ts
- packages/react/components/progress/Progress.tsx
🔇 Additional comments (9)
packages/react/components/progress/radial/ProgressRadial.tsx (1)
68-68: LGTM! Good type safety improvement
The explicit number type for animationFrameId correctly matches the return type of requestAnimationFrame. This improves type safety without needing eslint-disable comments.
packages/react/components/input/Input.tsx (1)
213-213: LGTM: Quote style consistency changes
The changes from double to single quotes align with the PR's objective of fixing lint warnings.
Also applies to: 223-223
packages/react/components/icon/IconProps.ts (2)
Line range hint 8-18: LGTM! Interface definition is well-structured.
The IconProps interface is properly typed with all necessary imports present for the defined properties.
1-3: LGTM! Removal of unused imports.
The removal of IconStatus and IconStatusValues imports aligns with fixing lint warnings, as these types are not used in the interface definition.
Let's verify there are no remaining references to these removed types:
packages/react/components/price/Price.tsx (1)
6-6: LGTM! Import cleanup aligns with lint fixes
The simplified import from '@/objects' improves code organization.
packages/react/components/price/Price.native.tsx (1)
Line range hint 7-12: LGTM! Import organization improved
The imports are now properly organized using the @/objects alias.
packages/react/objects/facets/Color.tsx (3)
32-32: Great type safety improvement!
Replacing Record<any, string[]> with Record<TrilogyColor, string[]> enforces proper type checking and removes the unsafe any type. This change aligns with TypeScript best practices.
131-131: Good use of enum value for type safety.
Using TrilogyColor.MAIN instead of string literal improves type safety and maintainability.
Line range hint 1-171: Verify remaining lint warnings.
Let's check if there are any remaining lint issues in the file.
✅ Verification successful
No significant lint issues found in Color.tsx
The file appears to be well-typed and follows TypeScript best practices:
- All enums, types, and function parameters have explicit type annotations
- Return types are properly specified
- The only
undefinedcheck is intentional for environment detection - No TODO/FIXME comments or lint-disable directives
- No usage of
anytype - Imports are properly typed and used
The only potential improvement would be to make the environment check more idiomatic, but it's not a lint issue.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for remaining lint warnings in the file
# Look for any TODO, FIXME, or lint-disable comments
rg -i "todo|fixme|lint|tslint|eslint" "packages/react/objects/facets/Color.tsx"
# Look for any 'any' types that might trigger lint warnings
rg "any" "packages/react/objects/facets/Color.tsx"
# Look for potential unused variables/imports
ast-grep --pattern 'import { $_ } from $_'
Length of output: 173
Script:
#!/bin/bash
# Let's try a more comprehensive analysis
# Check TypeScript-specific patterns that might indicate lint issues
echo "=== Checking function parameters without explicit types ==="
ast-grep --pattern 'function $name($params) { $$$}'
echo "=== Checking variable declarations without types ==="
ast-grep --pattern 'const $name = $_'
echo "=== Checking imports ==="
ast-grep --pattern 'import { $_ } from "$_"'
ast-grep --pattern "import { $_ } from '@_'"
echo "=== Checking function return types ==="
ast-grep --pattern 'function $name($_): $_ { $$$}'
ast-grep --pattern 'const $name = ($_): $_ => $_'
echo "=== Checking for potential undefined checks ==="
rg "undefined" "packages/react/objects/facets/Color.tsx"
echo "=== Checking ESLint configuration ==="
fd -e .eslintrc -e .eslintrc.js -e .eslintrc.json
Length of output: 26683
Summary by CodeRabbit
Release Notes
New Features
Selectcomponent for improved user experience.Bug Fixes
Inputcomponent during text input.Documentation
Style
Chores
Refactor
ModalandLinkcomponents for better maintainability.Tests
RadioandProgressRadial.