-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat(budget-tool): custom card drawing input #270
Conversation
…t-custom-drawing
@FaithDaka - thanks for sharing the draft PR, it took a bit of working out but I think I finally figured out what the issue is and have opened PR #271 targeting this branch to fix the issue. Feel free to merge it in if it makes sense to you. In terms of next steps I think the main task will be to support multiple paths, so that when the user releases their pointer the path is finalised and the user can start a new path on next mouse down. The svg element will then need to use an If you think that is better for a follow-up PR I think you could just comment out the element on the current budget testing page and we should be able to merge this in as a base component and build on in follow-up issue. We may also want to consider some stretch goals like an undo/redo button for drawn paths, as well as a clear button to start new. Further features like colour picker, eraser would probably be much lower priority. |
Screen.Recording.2024-05-27.at.10.00.42.mov |
Fixed the svg path bug. Screen.Recording.2024-05-28.at.21.24.48.mov |
feat: optimise svg drawing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @FaithDaka - working much better now, also like the undo/redo feature you've included.
I started testing and noticed a few performance issues on mobile. I've addressed some of them by merging in #284
There's still a few minor updates I want to add as part of the review process so will try come back to it later today.
- fix storing final path for future render/edit
- further testing on mobile
- remove test button and integrate into budget tool card pages
But overall I think looking really nice do I don't think any more work will be required from your side. Thanks again
Feat/custom drawing integration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've now merged #285 to integrate directly into the budget tool custom card drawing component and tidied up the output handling a bit - so I think should now be good to go (updated video in main description). Thanks again for all the help on this one
Description
Adds new drawing input component and integrates with budget tool custom cards
closes #230
Preview
Link to app preview if relevant
Screenshots / Videos
Screenity.video.-.Jun.5.2024.webm