Skip to content
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

Add preferNative option #77

Merged
merged 3 commits into from
Jul 26, 2023
Merged

Conversation

lucasweng
Copy link
Contributor

Provide an option to opt out of the native CSS text-balancing when the text-wrap: balance can't serve the needs or a customized ratio is preferred.

Closes #75.

@vercel
Copy link

vercel bot commented Jul 15, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-wrap-balancer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 21, 2023 0:38am

Copy link
Owner

@shuding shuding left a comment

Choose a reason for hiding this comment

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

Can you add the same option to Provider as well? And adjust the provider to pass it down. Thanks!

src/index.tsx Outdated
@@ -176,24 +191,25 @@ const Balancer = <ElementType extends React.ElementType = React.ElementType>({
}: BalancerProps<ElementType>) => {
const id = useId()
const wrapperRef = React.useRef<WrapperElement>()
const hasProvider = React.useContext(BalancerContext)
const contextValue = React.useContext(BalancerContext)
const preferNativeBalancing = preferNative && contextValue.preferNative
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @shuding,
Should Balancer be able to override the preferNative context value when the provider passes false? It seems to me overriding the true value from context is more likely to happen, as the case mentioned in #75 🤔 So, it only allows overriding preferNative: true for now. What are your thoughts on this? Thanks!

Copy link
Owner

Choose a reason for hiding this comment

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

Yes I think it should be able to override. Maybe we can extend this context to be an array or an object to contain more information if necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated Balancer in b9ceaa1.

If preferNative is not provided to Balancer, the ?? operator will take the value from the context, which defaults to true in createContext and in Provider. So when preferNative is explicitly provided, it can override.

@@ -184,7 +184,7 @@ const Provider: React.FC<{

 const Balancer = <ElementType extends React.ElementType = React.ElementType>({
   ratio = 1,
-  preferNative = true,
+  preferNative,
   nonce,
   children,
   ...props
@@ -192,7 +192,7 @@ const Balancer = <ElementType extends React.ElementType = React.ElementType>({
   const id = useId()
   const wrapperRef = React.useRef<WrapperElement>()
   const contextValue = React.useContext(BalancerContext)
-  const preferNativeBalancing = preferNative && contextValue.preferNative
+  const preferNativeBalancing = preferNative ?? contextValue.preferNative
   const Wrapper: React.ElementType = props.as || 'span'

Copy link
Owner

@shuding shuding left a comment

Choose a reason for hiding this comment

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

Thank you!

@shuding shuding merged commit c4d2b36 into shuding:main Jul 26, 2023
2 checks passed
@lucasweng lucasweng deleted the prefer-native-option branch July 26, 2023 06:27
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.

Option to force logic whether browser supports text-wrap: balancer
2 participants