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

js/ directory is not properly structured #182

Closed
pixelzoom opened this issue Nov 9, 2015 · 5 comments
Closed

js/ directory is not properly structured #182

pixelzoom opened this issue Nov 9, 2015 · 5 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

Related to code review #173.

The js/ directory currently has this structure:

gravity-and-orbits/
  model/
  module/
  view/
gravity-and-orbits-config.js
gravity-and-orbits-main.js

doc/implementation-notes.md describes the atypical structure of this sim, so I understand why there's a modules/ subdirectory. But I don't understand why that affects the ability to have a subdirectory per screen.

So while it's likely to be painful, it's recommended to restructure the source to match PhET conventions. There should still be a subdirectory per screen, plus a subdirectory for common (shared) things, like this:

cartoon/
    model/
    module/
    view/
common/
    model/
    module/
    view/
toScale/
    model/
    module/
    view/
gravity-and-orbits-config.js
gravity-and-orbits-main.js
@pixelzoom
Copy link
Contributor Author

While we're looking at the directory structure, I see currently see this:

gravity-and-orbits/
  view/
    bottom-control-panel/
    right-control-panel/
...

Both panels are on the right, so "right-control-panel" is ambiguous. One panel is on top, one is on the bottom. Recommended to rename "right-control-panel" to "top-control-pane". Or better yet, give these panels names indicative of their contents.

@aaronsamuel137
Copy link
Contributor

I don't understand why that affects the ability to have a subdirectory per screen.

There are really not many screen specific files in this sim. The two screens are nearly identical, and use the same files with a bit of conditional logic. Perhaps we could put CartoonModeList and RealModeList into different folders, but those are literally the only files, the rest would be common. Do you think its worth it to do that? I could just document more thoroughly why the directory structure is atypical.

Both panels are on the right, so "right-control-panel" is ambiguous.

These folders are badly named. The files in "right-control-panel" include both the control panels on the right. The files in "bottom-control-panel" are all the controls on the bottom of the screen, though they are not actually in a panel. What would you think about renaming the directories "right-controls" and "bottom-controls"? Or just moving everything to view?

@pixelzoom
Copy link
Contributor Author

Perhaps we could put CartoonModeList and RealModeList into different folders, but those are literally the only files, the rest would be common. Do you think its worth it to do that?

Yes.

What would you think about renaming the directories "right-controls" and "bottom-controls"? Or just moving everything to view?

I would move the files to view/. And deal with choosing better names in #192.

@aaronsamuel137
Copy link
Contributor

Restructured the files as suggested. Assigning to @pixelzoom for review.

@pixelzoom
Copy link
Contributor Author

👍 Closing.

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

No branches or pull requests

3 participants