Add minSize to circle.ts and rect.ts to prevent drawing unintentionally tiny shapes

[hiankun]

I found it might be more reasonable to limit the circle and the rectangle to their minimal size by the width of stroke width. Without the limitation, I have been occasionally added some "very tiny" circles or rectangles on the canvas and didn't aware it.

I have intermediate Python experience but nothing about web apps nor typescript (very beginner understanding about JS). Therefore I tried to modify the code by guessing the rules from the original code base, and there are probably some bad programming styles or practices.

Also this is my first PR. I've tried to get rid of the unwanted files (i.e., the folder named test_app/) but got no luck. In short, only circle.ts and rect.ts are relevant in this PR. If there are anything I can study from please also let me know.

[hiankun]

Wow, thank you so much for such a reply with so many information. I believe that your suggestions are clear but I just haven't had enough Git and Typescript knowledge to get them quickly. I will study them and please bear with me and my future questions.

[hiankun]

I think I have finished the code with slightly more consistent logic and style (thanks for the suggestions again).

Unfortunately I also messed up my forked repo.

Should I close this and open a new one so that I can give a cleaner PR? (I probably would re-fork a new repo to avoid all the mess...)

[hiankun]

To the question of transforming a created circle: Yes, the transform will not be limited by the minimal size.

(I also noticed that the transform don't preserve the stroke width. If possible, it would be nice to have an option to keep the stroke width while transforming the shapes. Of course it would be another topic.)

[andfanilo]

Unfortunately I also messed up my forked repo.

Should I close this and open a new one so that I can give a cleaner PR? (I probably would re-fork a new repo to avoid all the mess...)

If you feel courageous, you could create a new branch on your forked repo from the base branch (before your own commits) and merge in the files from your other branch, then recreate/edit the branch to merge in the PR..

If you feel less courageous, then yeah reforking, overwriting with the correct changes and closing -> recreating a PR is totally ok!

(I also noticed that the transform don't preserve the stroke width. If possible, it would be nice to have an option to keep the stroke width while transforming the shapes. Of course it would be another topic.)

Yup we would need to check Fabric.js options for this and see if they enable transform with stroke preservation, we can create an issue for this if it's something you need

Thanks for all the work!

[hiankun]

Hi, thank you for the workflow guidance, which has been what I need. I think I have fixed the PR (as it now shows only the two changed files). Please take a look and let me know whether I did it right or not.

[andfanilo]

Hello @hiankun Finally had a look at your changes locally and it looks good!

I'm ready to merge, but for this we need 2 last things:

• Could you format your code with Prettier? I usually have VSCode do it on Save as you write, but if you're using another IDE then doing npx prettier --write src\lib\rect.ts and npx prettier --write src\lib\circle.ts at streamlit_drawable_canvas\frontend level should do
• Your PR is still in draft mode so I can't merge it :upside_down_face:

Have a nice day, Fanilo :balloon:

[hiankun]

Hi @andfanilo ,

Thanks again for all the suggestions and instructions from which I've learned a lot. :-D