-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Break up Toolbar
elements
#2239
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
Yeah, definitely this would allow us to use these components separately in different places, this is really needed. Thanks @lindapaiste |
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.
instead of having a single file for the toolbar and exporting all the functions from a single file can we create a single folder inside of the components folder i.e. components/Toolbar
inside of which we can create separate files for all the 6 components and an index file for the toolbar itself?
@lindapaiste what do you think, which approach is better? In my opinion, this would help structure the project better.
I think this is a good idea. Will do. Sometimes I try to keep things in the same file as they were before for the sake of git history, so we can look back and see what changed when, but I changed so much that none of the lines match up anyways. 😝 |
@raclim @dewanshDT I have it broken up into separate files but Git is considering the original |
This reverts commit 776fe2a.
# Conflicts: # client/modules/IDE/components/Toolbar.jsx
Changes:
Break up the Toolbar into 6 independent reusable components. Each component selects its own state from Redux.
I also had to change the Toolbar tests to keep them passing.
Includes the Sidebar changes from #2233 because I'm using the
selectCanEditSketch
selector from that PR.Kind of a proof of concept. The idea is that we can export tiny pieces of the UI and use them in mobile components.
@dewanshDT do you think it's helpful to break things up this way?
@shujuuu's designs include a

PlayButton
which is the same button as on the web, but in a different place:and also an

EditableProjectName
(needs styling overrides) andProjectOwner
:I have verified that this pull request:
npm run lint
)npm run test
)develop
branch.Fixes #123