Closed lerrybe closed 1 year ago
I believe changing cursor when setBrushTool
is called would be better. I have checked some examples of other repositories.
tldraw
react-artboard
What was interesting is that tldraw did not use any custom svg cursors! Though the app itself has lots of tools, the developers seem to have used the css-supported basic cursors. tldraw only uses crosshair
for drawing tools. Tldraw sets the cursors when tool is changed.
React-artboard seems to use a similar approach. They use crosshair
for pen tool and a custom created circleCursor
for other brushes. Check this cursor.ts
if you would like to find out how they implemented their own custom cursor. It seems that they have embedded the svg inside the typescript. That is actually one way you could use for custom cursors. React-artboard also sets the cursor when the tool is changed. React-artboard implemented the functionality to change brushes using hooks like useMarker
, useWaterMarke
and so on. When that hook is called cursor is changed.
You can also reference toonie.ts
to see how you can implement svg into a typescript variable. When there is a need to implement svgs into a project, I would go to (https://react-svgr.com/playground/?typescript=true0) and then copy and paste the original raw svg. The site returns a compressed version of the svg so that the code does not get too long. It comes in handy when you just want to embed the svg in the code!
It is totally up to you to decide whether we will use svgs for our styling our cursors in the dotting library. Personally I do not mind using a simple crosshair
for all our drawing tools (EVEN tldraw crosshair
for all their drawing tools). However, if you would like users to customize their cursors, using embedded svg would be a great choice. If you think this cursor work seems like a cumbersome work, you may proceed to just use basic css cursors!
By the way, great job!
If you decided to default css style IMO deleting unused svg files would be better. otherwise LGTM +a) in UX approach tldraw's virtual cursor for erasor looks great. would there be no need for us to use virtual cursor? for example all same crosshair for different brush mode seems not to be the best
@Lee-Si-Yoon The cursor style can be later updated while @lerrybe works on issue #13 which is about using strategy pattern for controlling brush tools. Plus, it seemed to me that this cursor styling is less important compared to other major issues. We could later leave this svg styling issue to other contributors who joins this proejct as a good first issue.
Thank you all for the great reviews!๐
You are free to merge this PR. It would be great if you could rebase and merge
.
@hunkim98 @Lee-Si-Yoon
As @hunkim98 suggested, I think cursor-style part can be included while applying strategy patterns to the brush tool. Thank you for leaving a great review!
Kudos, SonarCloud Quality Gate passed!
๐ [Related Issue: #14]
Preview
https://github.com/hunkim98/dotting/assets/71599639/d8616b12-618c-4885-a4ab-0719132ea0f7
Changes
Implement cursor style when mouse move
Write a title of your change
[๐Other] implements styleMouseCursor function
Canvas/Editor.tsx
Notes
Question
Which is better to implement?
Next Up?