Skip to content
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

Structure #1

Merged
merged 5 commits into from
Jun 22, 2015
Merged

Structure #1

merged 5 commits into from
Jun 22, 2015

Conversation

eanplatter
Copy link
Contributor

Hey, so this is a proposal for how the file structure should be. Essentially we would have the App.js render all of the components on a sample page, and all of the components live within their own folder within the components directory.

Feedback is required ;)

@levithomason
Copy link
Member

Nice, this looks good so far. I removed most of my comments after reviewing our react style guide in more detail. Only survivor:

The directory components/TitleComponent seems a bit redundant. But I'm not sure what you had in mind for components with many moving parts (constants, stores, etc.)

I think we'd benefit by dropping in a component that requires more moving parts. This way we can have that structure nailed when starting real components.

Thoughts?

@eanplatter
Copy link
Contributor Author

Yeah, I was thinking for the file structure something like the following:

components
│
└─── StarRating
    │   StarRating.js (the actual component)
    │   Utils.js (any helpers needed)
    │
    └───Title
    │   Title.js (the actual component)

I honestly don't think stardust components will need anything too crazy, like a flux cycle. They should receive their state from props mostly.

@levithomason
Copy link
Member

Gotcha. OK, let's roll with what you outlined there then, dropping the redundant name. Once we get to building components we can iterate. ITERATE! :D

Edit

...emoji pending

@levithomason
Copy link
Member

Killin' it
           _,-,-.
        ,-: |.'-:|..-.
   _..-:| | `-`. \.--'
 <...--:'-|_|_|;  \
        \   /     /
         :      ,'
         |      |
         |      |
         |      |
         |      |

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants