Skip to content

Conversation

@OnestarLee
Copy link
Collaborator

@OnestarLee OnestarLee commented May 7, 2025

External Contributions

This project is not yet set up to accept pull requests from external contributors.

If you have a pull request that you believe should be accepted, please contact
the Developer Relations team [email protected] with details
and we'll evaluate if we can setup a CLA to allow for the contribution.

For Internal Contributors

[CLNP-6724](https://sendbird.atlassian.net/browse/CLNP-6724)

Description Of Changes

The borderRadius type in StyleSheet was updated to support string values in React Native 0.75.
To reflect this change, the parameter and return types were updated from
NonNullable<AnimatableNumericValue | undefined> to
NonNullable<AnimatableNumericValue | string | undefined>.

Additionally, // @ts-ignore was added to ensure compatibility with all supported React Native versions.

Types Of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply_

  • Bugfix
  • New feature
  • Documentation (correction or otherwise)
  • Cosmetics (whitespace, appearance (ex) Prettier)
  • Build configuration
  • Improvement (refactor code)
  • Test

@codecov-commenter
Copy link

codecov-commenter commented May 7, 2025

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 11.49%. Comparing base (7de7a89) to head (3d02958).

Files with missing lines Patch % Lines
...t-native-foundation/src/styles/createStyleSheet.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main     #240    +/-   ##
========================================
  Coverage   11.49%   11.49%            
========================================
  Files         358      358            
  Lines        8489     8489            
  Branches     2375     2257   -118     
========================================
  Hits          976      976            
  Misses       7512     7512            
  Partials        1        1            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@OnestarLee OnestarLee changed the title fix: allow string values for borderRadius in StyleSheet [CLNP-6724] fix: allow string values for borderRadius in StyleSheet May 7, 2025
@OnestarLee OnestarLee requested review from bang9 and Copilot and removed request for bang9 May 7, 2025 01:59
Copy link
Contributor

Copilot AI left a 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 updates the type definition for borderRadius in the StyleSheet to allow string values, reflecting changes in React Native 0.75. The changes include:

  • Updating the parameter and return types for DEFAULT_SCALE_FACTOR_WITH_NUMERIC_VALUE.
  • Adding a //@ts-ignore directive for the 'borderRadius' property.
  • Ensuring compatibility across supported React Native versions.

const preProcessor: Partial<StylePreprocessor> = {
'fontSize': DEFAULT_SCALE_FACTOR,
'lineHeight': DEFAULT_SCALE_FACTOR,
//@ts-ignore
Copy link

Copilot AI May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider adding an explanatory comment alongside //@ts-ignore to clarify the specific React Native compatibility issue being addressed.

Suggested change
//@ts-ignore
// @ts-ignore: React Native's type definitions do not fully align with the expected type for 'borderRadius'.

Copilot uses AI. Check for mistakes.
@OnestarLee OnestarLee requested review from bang9 and Copilot May 7, 2025 02:03
Copy link
Contributor

Copilot AI left a 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 updates the borderRadius type in StyleSheet to support string values, following the change in React Native 0.75.

  • Updated DEFAULT_SCALE_FACTOR_WITH_NUMERIC_VALUE to accept both numeric and string types.
  • Added a //@ts-ignore comment for the borderRadius preprocessor to support all active React Native versions.
Comments suppressed due to low confidence (1)

packages/uikit-react-native-foundation/src/styles/createStyleSheet.ts:15

  • [nitpick] Since the function now supports string values, consider renaming DEFAULT_SCALE_FACTOR_WITH_NUMERIC_VALUE to better reflect its functionality, for example, DEFAULT_SCALE_FACTOR_WITH_NUMERIC_OR_STRING_VALUE.
const DEFAULT_SCALE_FACTOR_WITH_NUMERIC_VALUE = (

const preProcessor: Partial<StylePreprocessor> = {
'fontSize': DEFAULT_SCALE_FACTOR,
'lineHeight': DEFAULT_SCALE_FACTOR,
//@ts-ignore : Ensure compatibility with all supported React Native versions
Copy link

Copilot AI May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider expanding the @ts-ignore comment to reference the relevant React Native version or commit for clarity and future maintainability.

Suggested change
//@ts-ignore : Ensure compatibility with all supported React Native versions
//@ts-ignore: Required for compatibility with React Native version >=0.65 due to changes in StyleSheet types. See https://github.com/facebook/react-native/commit/<commit-hash>.

Copilot uses AI. Check for mistakes.
@OnestarLee OnestarLee added this pull request to the merge queue May 7, 2025
Merged via the queue into main with commit f6be412 May 7, 2025
7 checks passed
@OnestarLee OnestarLee deleted the fix/support-border-radius-type branch May 7, 2025 04:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants