Skip to content

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

Closed
wants to merge 7 commits into from

Conversation

lindapaiste
Copy link
Collaborator

Changes:
Break up the Toolbar into 6 independent reusable components. Each component selects its own state from Redux.

  1. PlayButton
  2. StopButton
  3. AutoRefreshCheckbox
  4. EditableProjectName
  5. ProjectOwner
  6. PreferencesButton

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:
image

and also an EditableProjectName (needs styling overrides) and ProjectOwner:
image


I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@lindapaiste lindapaiste requested a review from dewanshDT June 11, 2023 00:16
@dewanshDT
Copy link
Collaborator

Yeah, definitely this would allow us to use these components separately in different places, this is really needed. Thanks @lindapaiste

Copy link
Collaborator

@dewanshDT dewanshDT left a 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.

@lindapaiste
Copy link
Collaborator Author

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. 😝

@lindapaiste
Copy link
Collaborator Author

lindapaiste commented Jun 14, 2023

@raclim @dewanshDT I have it broken up into separate files but Git is considering the original Toolbar.jsx and Toolbar.test.jsx as "deleted". It would be better to merge it in two phases (one to change contents, the other to move) in order to keep the Git history. I might rollback the last commit and put it in a separate PR.

# Conflicts:
#	client/modules/IDE/components/Toolbar.jsx
@lindapaiste lindapaiste marked this pull request as ready for review July 23, 2023 16:35
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