search

hyrwork / react-native-row

A wrapper around the react-native View component enabling concise assignment of flexbox properties
MIT License
51 stars 5 forks source link

Margin/Padding String shorthand (Idea) #12

Closed eightyfive closed 5 years ago

eightyfive commented 6 years ago

Just an idea.

I find it easier and more readable to write:

<View margin="20 15 10" />

Writing the shorthand like this is one String.split / Array.map / parseInt away from the original/needed array.

But one drawback is that we loose that use case:

Shorthand Style Result
margin={[20]} {marginVertical: 20}

Writing <View margin="20" /> would end up in margin = [20] but really it should be margin = 20.

There are 2 solutions to this problem:

  1. Pass a number instead of a string when you want "full" margin/padding: <View margin={20} /> and <View margin="20" /> remains margin = [20]
  2. Or always force vertical & horizontal values in shorthand, then <View margin="20" /> becomes the full margin, and if you want vertical only: <View margin="20 0" />

I'm leaning toward solution 2 because it's in line with CSS shorthand syntax. Not to mention that solution 2 would be a clearly separated syntax choice on its own, not overlapping with original Number/Array syntax.

c-moss-talk commented 5 years ago

@eightyfive I'm no longer at Hyr but I will see if @LuisRizo has time to look this over.

eightyfive commented 5 years ago

Thanks guys, FYI I started a branch with a couple of improvements: https://github.com/eightyfive/react-native-spacing

LuisRizo commented 5 years ago

@eightyfive Great work on that repo. Would you like to open a PR with the features that you wrote in your repo? I can merge it into this repo if you'd like

eightyfive commented 5 years ago

Hey,

I changed things quite a bit so I am not sure the PR would be readable nor easy to assess. For ex I use prettify for code formatting and I moved files around... So I am not sure what's the best/cleanest way to merge those changes.

Any recommandation?

eightyfive commented 5 years ago

Anyway I moved away from the idea of having the margin/padding helpers as props.

I separated the whole spacing strategy into a new standalone helper: https://github.com/eightyfive/react-native-spacesheet

And cleaned the react-native-row implementation to focus only on the dial idea: https://github.com/eightyfive/react-native-col

Proper credits added.