-
-
Notifications
You must be signed in to change notification settings - Fork 606
chore(Split): reorganize Split component exports and naming #3487
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
Conversation
I've removed exports for `SplitViewHost` & `SplitViewScreen` in favour of `Split.Host` & `Split.Column` / `Split.Inspector`. The naming here is open for discussion. I prefer short version of `Split` instead of `SplitView`, but that's matter of preference, can't find reasonable (& clinching) arguments for one or the other. Most important thing here is to just make it consistent. If we are to use `View` suffix, then we should do similar thing for Stack -> `StackView.Host` & `StackView.Screen`. So it's either: `Stack.Host`, `Stack.Screen`, `Tabs.Host`, `Tabs.Screen`, `Split.Host`, `Split.Column`, `Split.Inspector` OR `StackView.Host`, `StackView.Screen`, `TabsView.Host`, `TabsView.Screen`, `SplitView.Host`, `SplitView.Column`, `SplitView.Inspector` OR MAYBE (?) `Stack.HostView`, `Stack.ScreenView`, `Tabs.HostView`, `Tabs.ScreenView`, `Split.HostView`, `Split.ColumnView`, `Split.InspectorView`
c46bba0 to
2860ff8
Compare
t0maboro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with dropping View suffix.
One more question, can we correct warnings like SplitView is supported only for iOS. Consider using an alternative layout for Android. to drop the suffix? (I cannot ask for that in file directly as it was moved without changes)
|
Yeah, you're right. Thanks for noticing this. I'll fix the warning messages. |
t0maboro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, I'd suggest updating it in:
- SplitHost.tsx, L79 :
This enables us to fully recreate the SplitView when necessary, ensuring the correct column configuration is always applied. - SplitHost.types.ts, L299
Inspector will be displayed on the trailing edge of the main (secondary) column (for expanded Split) or as a modal (for collapsed SplitView).
apps/src/tests/TestSplit/index.tsx
Outdated
| const splitViewBaseConfig: SplitBaseConfig = { | ||
| preferredDisplayMode: 'twoBesideSecondary', | ||
| preferredSplitBehavior: 'tile', | ||
| }; | ||
|
|
||
| return ( | ||
| <SplitBaseApp splitBaseConfig={splitViewBaseConfig} /> | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const splitViewBaseConfig: SplitBaseConfig = { | |
| preferredDisplayMode: 'twoBesideSecondary', | |
| preferredSplitBehavior: 'tile', | |
| }; | |
| return ( | |
| <SplitBaseApp splitBaseConfig={splitViewBaseConfig} /> | |
| ); | |
| const splitBaseConfig: SplitBaseConfig = { | |
| preferredDisplayMode: 'twoBesideSecondary', | |
| preferredSplitBehavior: 'tile', | |
| }; | |
| return ( | |
| <SplitBaseApp splitBaseConfig={splitBaseConfig} /> | |
| ); |
|
This is back to draft for some time, as thanks to some recent commit the |
|
Works now |
@t0maboro this one is left deliberately, as this one refers to the |
Description
Closes https://github.com/software-mansion/react-native-screens-labs/issues/735
I'm open to discussion on the naming here.
This PR changes only JS layer of the component naming. If necessary native-side will be handled separately.
Changes
Export split component as
Splitinstead of separate onesI've removed exports for
SplitViewHost&SplitViewScreenin favourof
Split.Host&Split.Column/Split.Inspector.The naming here is open for discussion. I prefer short version of
Splitinstead of
SplitView, but that's matter of preference, can't findreasonable (& clinching) arguments for one or the other.
Most important thing here is to just make it consistent. If we are to
use
Viewsuffix, then we should do similar thing for Stack ->StackView.Host&StackView.Screen.So it's either:
Stack.Host,Stack.Screen,Tabs.Host,Tabs.Screen,Split.Host,Split.Column,Split.InspectorOR
StackView.Host,StackView.Screen,TabsView.Host,TabsView.Screen,SplitView.Host,SplitView.Column,SplitView.InspectorOR MAYBE (?)
Stack.HostView,Stack.ScreenView,Tabs.HostView,Tabs.ScreenView,Split.HostView,Split.ColumnView,Split.InspectorViewUpdate examples
Remove
viewsubstring fromSplitViewprefix used across symbols