feat: updated component directory structure #163
Conversation
| @@ -23,7 +22,7 @@ interface ScreenProps { | |||
| /** The content to render within the screen */ | |||
| children?: ReactNode; | |||
| /** Whether to force the topInset. Use to prevent screen jank on tab screens */ | |||
| forceTopInset?: Boolean; | |||
There was a problem hiding this comment.
@dayhaysoos I'm assuming react-navigation fixed something here and this is no longer required?
There was a problem hiding this comment.
@cball I actually have no idea why that went missing. I think @isaiahgrey93 knows why it's gone
ryanpatk
changed the title
ryanpatk
force-pushed
the
nick/rfc-component-structure
branch
from
74ba02e to
1a81069
Compare
There was a problem hiding this comment.
Changes:
- I don't think we want a
yarn.lockintemplate/?
A few questions:
- does AppNav render ok without a real user?
And notes:
- I added a link to the eslint plugin for import sorting. If we're doing that we definitely need it to be in the plugin rather than relying on manual changes
| @@ -21,7 +21,7 @@ const App = (): ReactElement | null => { | |||
|
|
|||
| const renderNavigation = (): ReactElement => { | |||
| // replace with actual user profile data once loaded, or fallback to guest nav | |||
| const currentUser = false; | |||
| const currentUser = true; | |||
There was a problem hiding this comment.
I'm assuming neither of these matter as they are meant to be replaced, but does AppNav render properly without a real user?
| import { colors } from '../styles'; | ||
| import { Container } from './Container'; | ||
| import { Text } from './Text'; | ||
| import { Touchable } from './Touchable'; |
There was a problem hiding this comment.
If we're going to sort these now, let's make sure to add import/order plugin to our react-native eslint plugin (I've created a followup issue here: echobind/eslint-plugin-echobind#141)
| @@ -1,7 +1,7 @@ | |||
| import { storiesOf } from '@storybook/react-native'; | |||
| import React from 'react'; | |||
|
|
|||
| import { Text } from './Text'; | |||
| import { Text } from '../../src/components/Text'; | |||
There was a problem hiding this comment.
this feels a bit odd including src in the import, but not a big deal
There was a problem hiding this comment.
Yeah, I didn't want to move the storybook folder from the root level, do you think we should add it to src?
The reason why I did this is because I would do this with tests as well, tests would replicate the src directory in structure but have nothing but test files.
For full context, refer to this link: https://www.notion.so/echobind/RFC-Different-approach-for-component-structure-dd0d2d308c4c4a60a58849acb4e65f8f
Overview
Component structure has been updated to get away from having directories that represent components. The previous structure looked something like:
Which has now been switched to:
It's worth mentioning that the storybook files have been moved to the storybook directory.
Lemme know if you have any trouble spinning this up or any other questions!