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

clear and undo buttons #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Niyatizzz
Copy link

fixes issue #14
building up on pr #15
clear button clears everything on the screen while the undo button clears only the drawing leaving the dots as required
do let me know if it requires further changes

@quozl
Copy link
Contributor

quozl commented Nov 2, 2024

Thanks. There was too much change. Please don't make changes to whitespace and don't commit your own files like settings.json.

https://github.com/sugarlabs/sugar-docs/blob/master/src/contributing.md#making-commits is our suggestions on commit messages.

How did you test the activity? Activities must be tested inside Sugar.

@Niyatizzz
Copy link
Author

I have cleared the extra whitespaces and have tested the activity on sugar as well

@quozl
Copy link
Contributor

quozl commented Nov 5, 2024

How did you test the activity?

Still the same amount of whitespace change, see https://github.com/sugarlabs/connect-the-dots-activity/pull/18/files for most of index.html and activity.js.

@chimosky please also review?

@chimosky
Copy link
Member

chimosky commented Nov 5, 2024

I agree with Quozl, there's just a lot of change to the syntax which makes your actual change difficult to review.

You should also take a look at our contributing guide, we usually don't open PRs from the master branch.

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.

3 participants