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

feat(budget-tool): custom card drawing input #270

Merged
merged 26 commits into from
Jun 5, 2024
Merged

Conversation

FaithDaka
Copy link
Collaborator

@FaithDaka FaithDaka commented May 13, 2024

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

@github-actions github-actions bot added the Tool: Budget Updates related to Budget tool label May 13, 2024
@FaithDaka FaithDaka self-assigned this May 13, 2024
@FaithDaka FaithDaka added the help wanted Extra attention is needed label May 13, 2024
@chrismclarke
Copy link
Collaborator

@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 @for loop to render each of the paths on the page. We will also want to replace the button component I've added with some sort of div/svg that renders the current svg, and when clicked opens up the svg editor.

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.

@chrismclarke chrismclarke removed their request for review May 13, 2024 21:42
@FaithDaka
Copy link
Collaborator Author

Screen.Recording.2024-05-27.at.10.00.42.mov

@FaithDaka
Copy link
Collaborator Author

Fixed the svg path bug.
Pending bug: undo and redo functionality

Screen.Recording.2024-05-28.at.21.24.48.mov

@FaithDaka FaithDaka marked this pull request as ready for review May 28, 2024 18:32
@chrismclarke chrismclarke self-requested a review June 4, 2024 07:20
Copy link
Collaborator

@chrismclarke chrismclarke left a 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

@chrismclarke chrismclarke changed the title Custom Drawing Tool (Perfect Freehand integration) feat(budget-tool): custom card drawing input Jun 5, 2024
@chrismclarke chrismclarke self-requested a review June 5, 2024 09:36
Copy link
Collaborator

@chrismclarke chrismclarke left a 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

@chrismclarke chrismclarke merged commit 582a8cf into main Jun 5, 2024
5 checks passed
@chrismclarke chrismclarke deleted the feat-custom-drawing branch June 5, 2024 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature help wanted Extra attention is needed Tool: Budget Updates related to Budget tool
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Feat: Custom card draw
2 participants