Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Style: style prop could directly accept array#750

MoOx started this conversation inIdeas
Discussion options

MoOx
Jul 12, 2021
Maintainer

At first, when we revisited bs-react-native, we had for style

  • single style
  • array of style
  • list of 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).
I would be interested to know about the community practise because I am thinking

"what if style prop only accept array of style?"

Currently we need to write:

  • single:<Text style={style}
  • multiple:<Text style={array([style, style1])} or<Text style={Style.array([style, style1])} (depending on opened modules)

If we change this, we would just have to type

  • single:<Text style={[style]}
  • multiple:<Text style={[style, style2]}

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)

You must be logged in to vote

Replies: 6 comments 8 replies

Comment options

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.

You must be logged in to vote
0 replies
Comment options

Same for me, I'll not be against this. But what aboutarrayOption ?

You must be logged in to vote
6 replies
@MoOx
Comment options

MoOxJul 15, 2021
Maintainer Author

Actually Style.t is used for everything, so aviewStyle is at, andarray(...) is t too, same forarrayOption. This means if we change style props fromStyle.t toarray(Style.t), you could still usearray andarrayOption.
Like said above, this is due to the fact that RN accepts nested arrays, so you can pass in JSstyle={[[{color: "blue"}], [[{"backgroundColor": "red"}]]]} and it will work.

@sgny
Comment options

sgnyJul 15, 2021
Maintainer

Exactly. Otherwise we would have already run into trouble seeing how helper functions allow recastingarray(Style.t) intoStyle.t.

I like this change, but I further propose definingStyle.style to returnarray<t> instead oft as below. Then, minimal refactoring would needed and we can still continue writingstyle=someStyle. The resulting type definitions would not be less precise in practice either.

style: (  ~resizeMode: resizeMode=?,  ...  ~zIndex: int=?,  unit,) => array<t> = ""
@MoOx
Comment options

MoOxJul 15, 2021
Maintainer Author

Oh that is actually an interesting idea but maybe that would allow people to write things that will create runtime error likesomeStyle->Array.length ?!

@sgny
Comment options

sgnyJul 15, 2021
Maintainer

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.

@jfrolich
Comment options

Yes this would not be safe

Comment options

I also would not be against this. Should be an easy refactor on the existing codebase.

You must be logged in to vote
0 replies
Comment options

MoOx
Sep 18, 2021
Maintainer Author

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?

You must be logged in to vote
0 replies
Comment options

Personally, I strongly prefer the way it is currently implemented.

I just checked one of our projects and, out of 320 usages ofstyle={, found just

  • 38 xStyle.array
  • 7 xStyle.arrayOption

Also, usage witharrayOption within an array would seem weird to me.

You must be logged in to vote
1 reply
@Freddy03h
Comment options

I'm still thinking style should accept an array directly

On my project:

944 usages ofstyle={

  • 395 xStyle.array
  • 64 xStyle.arrayOption

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, …

Comment options

MoOx
Oct 12, 2022
Maintainer Author

Interesting stuff are in this PR: we tried but there is some complications: runtime & futur proof issues?

#769

@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

You must be logged in to vote
1 reply
@Freddy03h
Comment options

Yeah I just remember I read something about it, but it say

accessing the returned style objects at runtime is still not recommended as it can prevent static extraction to CSS

so my point about using spread operator is still irrelevant.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Category
Ideas
Labels
None yet
6 participants
@MoOx@jfrolich@cknitt@Freddy03h@gedeagas@sgny

[8]ページ先頭

©2009-2025 Movatter.jp