-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Merge changes from vikalp elements #3984
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Types with string literals and `string` are collapsed to `string` by TypeScript.
By changing the `string` to `stiring & {}` we can avoid this.
…nd adjust tsconfig settings
…x, ListItem, Slider, Tab, and TabView components
#9 (#51) * fix(Tab): center selected tab and fix scroll positioning (react-native-elements#3740) * fix(Tab): add LayoutChangeEvent import for improved layout handling * fix(Tab): Code style issues. Fixed with Prettier. * fix(Tab): clean up code formatting for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request merges changes from the "vikalp elements" branch that includes fixing broken GitHub Actions, upgrading React Native and Expo versions, and adding scripts for documentation management and version upgrades for admins. The changes primarily focus on modernizing the build tools, fixing cross-platform compatibility issues, updating dependencies, and improving the development workflow with better scripts and updated configurations.
Key Changes:
- Build system modernization: Upgraded React Native, Expo, TypeScript, and Jest to newer versions with updated configurations
- Cross-platform compatibility fixes: Replaced Node.js
pathmodule withposixequivalent for consistent path handling across platforms - Development workflow improvements: Added scripts for documentation management, version upgrades, and better error handling in release processes
Reviewed Changes
Copilot reviewed 160 out of 196 changed files in this pull request and generated 8 comments.
Show a summary per file
:
| File | Description |
|---|---|
| scripts/release/index.ts | Enhanced release script with better error handling, documentation management, and website update automation |
| scripts/docgen/src/ | Updated documentation generation with posix path handling and improved template rendering |
| packages/base/src/ | Updated components with modern React Native patterns, fixed prop handling, and improved TypeScript types |
| packages/themed/ | Updated Jest configuration, test imports, and package.json with new dependency versions |
| package configuration files | Updated Babel presets, Jest setup, and dependency versions across multiple packages |
| }); | ||
| } | ||
|
|
||
| static async updateWebsiteDocs(oldVersion: string, newVersion: string): Promise<void> { |
Copilot
AI
Jul 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is quite large (52 lines) and handles multiple responsibilities. Consider breaking it down into smaller, focused methods like handleVersionChange, updateVersionedDocs, and updateVersionsJson for better maintainability.
| //@ts-ignore | ||
| toValue: toValue, | ||
| useNativeDriver: true, | ||
| easing: Easing.inOut(Easing.ease), | ||
| duration: 150, | ||
| ...animationConfig, |
Copilot
AI
Jul 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using @ts-ignore without explanation suppresses TypeScript safety. Add a comment explaining why this is necessary or properly type the animation configuration.
| //@ts-ignore | |
| toValue: toValue, | |
| useNativeDriver: true, | |
| easing: Easing.inOut(Easing.ease), | |
| duration: 150, | |
| ...animationConfig, | |
| toValue: toValue, | |
| useNativeDriver: true, | |
| easing: Easing.inOut(Easing.ease), | |
| duration: 150, | |
| ...(animationType === 'timing' | |
| ? (animationConfig as Animated.TimingAnimationConfig) | |
| : (animationConfig as Animated.SpringAnimationConfig)), |
| export type SearchBarRef = { | ||
| focus: () => void; | ||
| blur: () => void; | ||
| clear: () => void; | ||
| cancel: () => void; | ||
| }; | ||
|
|
||
| return ( | ||
| <Component | ||
| ref={(ref: SearchBarIOS) => { | ||
| this.searchBar = ref; | ||
| }} | ||
| {...this.props} | ||
| /> | ||
| ); | ||
| } | ||
| } | ||
| export const SearchBar = forwardRef< | ||
| SearchBarRef, | ||
| SearchBarProps & { theme?: Theme } | ||
| >((props, ref) => { | ||
| const { platform = 'default' } = props; | ||
| const searchBarRef = useRef<SearchBarRef>(null); | ||
|
|
||
| useImperativeHandle(ref, () => ({ | ||
| focus: () => searchBarRef.current?.focus(), | ||
| blur: () => searchBarRef.current?.blur(), | ||
| clear: () => searchBarRef.current?.clear(), | ||
| cancel: () => searchBarRef.current?.cancel(), | ||
| })); | ||
|
|
||
| const Component: React.ElementType = | ||
| SEARCH_BAR_COMPONENTS[platform] || SearchBarDefault; | ||
|
|
||
| return <Component ref={searchBarRef} {...props} />; | ||
| }); |
Copilot
AI
Jul 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The component was converted from class to functional component but the ref interface only provides imperative methods. Consider if this aligns with React's recommended patterns for component design.
|
Hi @deepktp, thank you for the work that went into this PR! I am excited to work with you to get this library back into a working state. I collaborated with @Monte9 on the review and noticed that when I ran this locally on the example app (Android emulator, Expo 52), I ran into an error with the Linear Progress component. I have attached a screenshot but the error is "Loss of precision during arithmetic conversion: (long) 0.3" I know you have made a lot more progress since this PR was initially proposed, so we can coordinate on a fix to this or perhaps pulling in further updates. Thanks!
|
Merge changes from vikalp elements
Merge changes from vikalp elements
theianmay
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged, thank you!

Uh oh!
There was an error while loading. Please reload this page.