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

Add stroke path to Path2D interface per html canvas spec #350

Merged
merged 10 commits into from
Sep 6, 2024
Merged

Conversation

cleemesser
Copy link
Contributor

@cleemesser cleemesser commented Sep 3, 2024

  • lacks tests
  • lacks documentation

Copy link
Contributor

github-actions bot commented Sep 3, 2024

lite-badge 👈 Try it on ReadTheDocs with JupyterLite!

@cleemesser cleemesser changed the title Add stroke path to Path2D interface per html canvas spec #334 Add stroke path to Path2D interface per html canvas spec Sep 3, 2024
@martinRenou
Copy link
Collaborator

Looks great!

lacks tests

This could be as simple as adding a new cell at the end of this Notebook testing the feature:
https://github.com/jupyter-widgets-contrib/ipycanvas/blob/master/ui-tests/tests/notebooks/ipycanvas.ipynb

@martinRenou
Copy link
Collaborator

CI failures look unrelated, I will fix them in parallel

* add stroke(path: Path2D) form to canvas

---------

Co-authored-by: Chris Lee-Messer <[email protected]>
@martinRenou
Copy link
Collaborator

Done and rebased. There is a linting failure that comes from this PR.

@cleemesser
Copy link
Contributor Author

Looks great!

lacks tests

This could be as simple as adding a new cell at the end of this Notebook testing the feature: https://github.com/jupyter-widgets-contrib/ipycanvas/blob/master/ui-tests/tests/notebooks/ipycanvas.ipynb

  • I've prepared a simple test for ui-tests/notebooks/ipycanvas.ipynb
  • I tried black --check on my repo files and don't get any changes. not sure what needs to be fixed from linking/formatting point of view. maybe look again soon

@martinRenou
Copy link
Collaborator

Thanks!

I tried black --check on my repo files and don't get any changes

black --check only checks without making changes (for testing the linting), you may want to run just black and commit the changes black makes.

@martinRenou
Copy link
Collaborator

bot please update snapshots

@cleemesser
Copy link
Contributor Author

cleemesser commented Sep 5, 2024

I went ahead and ran black again on canvas.py and pushed the commit.
BTW: I noticed that black wants to a lots of reformats on the ui-test ipycanvas.ipynb notebook. I did not include those changes in my submission, but could in a future PR..

..oops my push failed at first because I had not resynced with upstream. now should be available

@martinRenou
Copy link
Collaborator

Thanks!

I went ahead and ran black again on canvas.py and pushed the commit.

I don't see your new commit, are you sure you pushed?

I did not include those changes in my submission, but could in a future PR

Sounds good!

@martinRenou
Copy link
Collaborator

bot please update snapshots

martinRenou

This comment was marked as resolved.

@martinRenou
Copy link
Collaborator

Triggering CI

@martinRenou martinRenou closed this Sep 6, 2024
@martinRenou martinRenou reopened this Sep 6, 2024
Copy link
Collaborator

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

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

Thanks!

@martinRenou martinRenou merged commit 37cf1ea into jupyter-widgets-contrib:master Sep 6, 2024
3 checks passed
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