Sketch Canvas Component#500
Conversation
|
Visit the preview URL for this PR (updated for commit 45dca66): https://nwplus-ubc--pr500-sketch-canvas-compon-cgz71128.web.app (expires Thu, 24 Aug 2023 09:25:41 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 8c7ea898e009e43455645bc310bcbccfc0f87e48 |
meleongg
left a comment
meleongg
left a comment
There was a problem hiding this comment.
AMAZING work Lincoln! SO SPEEDY! I did not know these libraries existed that could do this lol
Your overall approach LGTM, just a couple additional comments:
- mobile responsiveness -> see ThemeProvider.js for booleans you can use within styled-components
- I think loading from Firebase would be the next step given that you haven't touched the "value" prop passed from Skills.js -> SketchCanvas.js
| <QuestionHeading>question 21</QuestionHeading> | ||
| <SubHeading size="1.25em">Draw your favourite character!</SubHeading> | ||
| <SketchCanvas | ||
| width={600} |
There was a problem hiding this comment.
I don't think this is mobile responsive right now. Looking at the npm package, it looks like react-sketch-canvas supports web & mobile
There was a problem hiding this comment.
done (i think). I didn't touch height cuz I personally think it looks better with the same height across all devices, but lmk if you think otherwise!
| const [showPicker, setShowPicker] = useState(false) | ||
| const canvasRef = useRef() | ||
|
|
||
| const [debounceTimerId, setDebounceTimerId] = useState(null) |
There was a problem hiding this comment.
Should we could extract the debouncer logic into its own hook to make things cleaner?
There was a problem hiding this comment.
see useDebounce in utilities.js w/i this repo
There was a problem hiding this comment.
done (i think). lmk if you want me to change more things
| const newTimerId = setTimeout(async () => { | ||
| if (!loadedRef.current) return | ||
|
|
||
| if (exportAsBase64) { |
There was a problem hiding this comment.
Not sure I follow why we want to save as SVG for the first save and then JPEG for the following saves.
DonaldKLee
left a comment
DonaldKLee
left a comment
There was a problem hiding this comment.
Overall, great work on this feature! Amazed by how fast you completed it :)
Some notes
- Clicking on "next" after drawing and then "back" resets the drawing
- is it possible to render a image onto their canvas? if not, a warning message might be helpful to let users know that they need to restart if they leave the page. Or you can also just display their data64 image below the canvas so applicants know that they did draw something
- Can we display their drawing on the "Review your submission" page?
- Not mobile responsive as Melvin mentioned
- Are we making the drawing required? (leaving canvas empty allows us to move on)
4 participants
Description