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

Move the toolpad app out of the components folder #657

Merged
merged 3 commits into from
Jul 14, 2022
Merged

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Jul 14, 2022

Follow up on #644
Moves toolpad application components it into a top-level ./toolpad folder so that we reserve the ./components folder for reusable react components.

For now kept the name ./toolpad since I feel like ./editor doesn't quite capture the breadth of what's in this folder (also has releases/app overview/...). But open for discussion

@render
Copy link

render bot commented Jul 14, 2022

@oliviertassinari oliviertassinari requested a deployment to toolpad-app - toolpad-db PR #657 July 14, 2022 12:56 — with Render Abandoned
@Janpot Janpot changed the title Move the toolpad app out of components folder Move the toolpad app out of the components folder Jul 14, 2022
Copy link
Member

@bharatkashyap bharatkashyap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2022-07-14 at 8 41 45 PM

I would think that a name that is more semantically precise - like the other folders - might be better; for instance, I wouldn't be able to understand why the files in the toolpad folder are grouped together whereas it is evident for all other folders.

Perhaps home or appShell?

@apedroferreira
Copy link
Member

Screenshot 2022-07-14 at 8 41 45 PM

I would think that a name that is more semantically precise - like the other folders - might be better; for instance, I wouldn't be able to understand why the files in the toolpad folder are grouped together whereas it is evident for all other folders.

Perhaps home or appShell?

or maybe just app? not sure if that's an improvement, just a possible suggestion.
anyway i think these folder changes are already an improvement.

@Janpot
Copy link
Member Author

Janpot commented Jul 14, 2022

The name "app" is too ambiguous, it could refer to the apps you create inside of toolpad. I'm thinking "designer", "workspace", "studio",...

@apedroferreira
Copy link
Member

The name "app" is too ambiguous, it could refer to the apps you create inside of toolpad. I'm thinking "designer", "workspace", "studio",...

I usually put the non-reusable components inside folders for each page of the website, in a pages folder - not sure how much that could apply to this app though since pages can have so many specific sections.
But anyway any of those names seem fine to me, or we can keep toolpad too for now :)

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 14, 2022
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 14, 2022
@Janpot Janpot merged commit a8d2cfc into master Jul 14, 2022
@Janpot Janpot deleted the toolpad-app branch July 14, 2022 20:57
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.

4 participants