khevamann / rn-responsive-styles

Responsive styles for react-native and react-native-web
MIT License
39 stars 3 forks source link

V2 #13

Closed khevamann closed 1 year ago

khevamann commented 1 year ago

This update changes the api so is a breaking change.

BREAKING CHANGES:

styles('container') should be updated to styles.container

Why

This provides much better type support as well as a more intuitive way to use styling that does not require the use of functions

codecov-commenter commented 1 year ago

Codecov Report

Merging #13 (ac43559) into main (b9d4983) will increase coverage by 1.78%. The diff coverage is 97.36%.

@@            Coverage Diff             @@
##             main      #13      +/-   ##
==========================================
+ Coverage   96.25%   98.03%   +1.78%     
==========================================
  Files           7        9       +2     
  Lines          80      102      +22     
  Branches       10        7       -3     
==========================================
+ Hits           77      100      +23     
  Misses          2        2              
+ Partials        1        0       -1     
Impacted Files Coverage Δ
src/hooks/useDeviceSize.ts 87.50% <87.50%> (ø)
src/createResponsiveStyle.ts 100.00% <100.00%> (ø)
src/helpers.ts 100.00% <100.00%> (ø)
src/hooks/useResponsiveStyle.ts 100.00% <100.00%> (ø)
src/index.ts 100.00% <100.00%> (ø)
src/provider.tsx 100.00% <100.00%> (ø)
src/types.ts 100.00% <100.00%> (ø)
tests/OverwriteStyle.tsx 100.00% <100.00%> (ø)
tests/Sample.tsx 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

khevamann commented 1 year ago

@EthanHermsey Would love to hear your thoughts on this update if you are still using this package

EthanHermsey commented 1 year ago

Oh Mann! I did not expect this update, and I'm glad you pinged me. I am indeed still using the package, even using it in a work project now because t's small and delivers nice functionality.

I love the new syntax! It's a lot less / faster typing without the brackets and quotes. For me it's a perfect middle way for the 'multiple styles' I proposed. And this way it's even more similar to RN's way of coding. I'm hoping my ide picks up the autosuggestion for the styles. ( it should )

I like the provider solution for the breakpoints. It would be nice to have more than 4, but I guess that would make naming them a bit more complicated. I was also thinking this; I usually have global styles ( containers, margins, text sizes ) and use a hook to load them. Then for certain components I create scoped styles with createResponsiveStyles ( and sometimes both ). It would be nice if you could provide the global styles to the provider so that they get mixed in when creating the scoped responsive styles in the components.

I'm not sure if it's possible.. It would mean you should also be able to get the global styles, even without creating scoped styles.. And what if you accidentally create styles with the same name? Would the autosuggestions still work? I thought I'd share it with you, maybe you find something interesting in there.

Thanks for the heads up, I'll give you a report if anything fails, but judging from the code that won't be likely :)

khevamann commented 1 year ago

@EthanHermsey Awesome! Yeah let me know if you run into any issues. I've migrated our work repo and it was pretty seamless just some fancy find and replace regex. The global styles is interesting. It would definitely work, I just wonder if it would be bad to be importing all the global styles everywhere throughout the app. Also yeah typescript support and autosuggestions wouldn't work unless there was an exported type. The best solution would probably be just to create an object of global styles and inject them in to components you need them in, something like this:

const styles = CreateResponsiveStyles({
    ...GlobalStyles,
    container: {
        flex: 1
    }
})

And yeah I was thinking about more breakpoints that but couldn't think of a great way to add more. The way I was thinking would be to pass an object to the provider

{
    "@small-tablet": 500,
    "@small-laptop": 800,
    "@small": 300
}

Then you could use those tags throughout the app instead of DEVICE_SIZES.SM, I just like the enum. But I actually think this would probably be a good improvement

EthanHermsey commented 1 year ago

1 I'm pretty sure I'm doing something wrong. It's soo slooooow ;p haha

This is right after I switched the syntax. My useStyles hook ( to get the global styles ) is acting up. I'll look into why later. It is used in pretty much all components, It seems like it has to do a lot of work, building styles maps.

The switch was indeed easily done with some regex ( thanks for sharing that website, helped a lot lately ).

I have to say, I kinda like this... { "@small-laptop": 800, "@small-tablet": 500, "@small": 300 }

khevamann commented 1 year ago

Interesting, I'm happy to take a look if you want to send me a sample later today. You can email me through my profile too if that's easier