Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork149
-
At first, when we revisited bs-react-native, we had for style
Recently, since array usage has been promoted in favour of list (we made this a breaking change in one of the last major release), so we now only have single style or array of style. I find myself using array of style more than half of the time (because I use a combination of static styles and dynamic styles (hello dark mode). "what if style prop only accept array of style?" Currently we need to write:
If we change this, we would just have to type
That would be obviously a breaking change, but should be easy to change. Thoughts? poke@jfrolich@cknitt@sgny@idkjs@Freddy03h@gedeagas@celsobonutti (and anyone who wants to give their feedback on this idea) |
BetaWas this translation helpful?Give feedback.
All reactions
👍 2
Replies: 6 comments 8 replies
-
Same feeling@MoOx, also since using reanimated2 I use array even more. I think this would be a net improvement. There might be a small performance regression when using a an array of 1 vs a single style, but probably diminishable. |
BetaWas this translation helpful?Give feedback.
All reactions
👍 2
-
Same for me, I'll not be against this. But what about |
BetaWas this translation helpful?Give feedback.
All reactions
-
Actually Style.t is used for everything, so a |
BetaWas this translation helpful?Give feedback.
All reactions
-
Exactly. Otherwise we would have already run into trouble seeing how helper functions allow recasting I like this change, but I further propose defining |
BetaWas this translation helpful?Give feedback.
All reactions
-
Oh that is actually an interesting idea but maybe that would allow people to write things that will create runtime error like |
BetaWas this translation helpful?Give feedback.
All reactions
-
That's a valid concern. Multiple styles would become a worse problem with this too. Joining an actual array and an element would be problematic. I suppose I haven't thought this through. |
BetaWas this translation helpful?Give feedback.
All reactions
-
Yes this would not be safe |
BetaWas this translation helpful?Give feedback.
All reactions
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
-
I also would not be against this. Should be an easy refactor on the existing codebase. |
BetaWas this translation helpful?Give feedback.
All reactions
-
Since time is limited, I have a simple solution (temporary) for us to try this at a large scale before pushing this as the main method: I was thinking we could add an alternate Style module (eg: Style2 or AStyle or a better (but short) name) to try this. Obviously if we keep Style that would means that there is at some point a weird magic trick to make them coexist but maybe it worth a try? Any thoughts? |
BetaWas this translation helpful?Give feedback.
All reactions
-
Personally, I strongly prefer the way it is currently implemented. I just checked one of our projects and, out of 320 usages of
Also, usage with |
BetaWas this translation helpful?Give feedback.
All reactions
-
I'm still thinking style should accept an array directly On my project: 944 usages of
Like@MoOx said, there is too much dynamic styles on a mobile today: light and dark mode, safe area insets, screen dimensions, ios vs android vs web, … |
BetaWas this translation helpful?Give feedback.
All reactions
-
Interesting stuff are in this PR: we tried but there is some complications: runtime & futur proof issues? @Freddy03h you mentioned that RNW could go back to num ids. But they just changed in 0.18https://github.com/necolas/react-native-web/releases/tag/0.18.0 |
BetaWas this translation helpful?Give feedback.
All reactions
-
Yeah I just remember I read something about it, but it say
so my point about using spread operator is still irrelevant. |
BetaWas this translation helpful?Give feedback.