Add Capture button to IDE toolbar to update main image of Part #209
Reference in New Issue
Block a user
Delete Branch "main"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Adds a button to the CascadeStudio IDE to capture the camera view, then allows user to download the screenshot and to update the Part's main image. Related to #111 and #208. Designs can be seen in Figma. Happy to make any adjustments or edits needed!
Possible improvement could be to set up automatically saving a screenshot on first save of a Part, so all projects have a screenshot.
Needs reworking to have setting the Part's image as the primary action, and to have tests written. Planning to work tonight or tomorrow (US Eastern).
@Irev-Dev I've updated so that the toolbar has a "Set Part Image" button instead of "Capture" per Figma feedback. This is ready for your review.
Pressing the button uploads and immediately overwrites whatever mainImage may have been set for the part, and then gives the user the opportunity to save a copy to their computer and shows them the image capture. This leads me to 2 questions:
Concerns
capture()method to yourcascadeControllerclass, but I don't know if that was the right place to put it. My thought was it is in a sense controlling CascadeStudio?IdeToolbar.js. Was that okay? Is there enough state in there that it should be transitioned to useuseReducer?position: fixedto style this popup, so it isn't affected by document flow?Future Work
I was unable to figure out how to automatically capture a screenshot on initial save of a Part: I need a reference to the part to complete my
onCapture()call, but after initial savenavigate()is called and appears to refresh the page and wipe the current state. Would love any help on this.This looks really good @franknoirot 🙌
You've done a good job of figuring out the structure i.e. putting the svgs with the rest of them. I like the cloundinary helper and I think putting the capture on the cascade studio controller is the perfect place for that, and it all done in only 160 odd lines of code.
Questions
I think you might be right, it does seem very sudden, I guess I chose poorly form your options on figma and the original option of having a popup that has "download" and "set part image" as options would be better? I think maybe keeping the copy for the tool bar button as
set part imageis still a good idea just to highlight that usecase? (also maybe lets change the worksettosaveorcreate. I think set is a little to programmer talk. What do you think?Yes the images should be destroyed, when the part is updated if the image id changes it tells Cloudinary to remove the old image. see https://github.com/Irev-Dev/cadhub/blob/main/api/src/services/parts/parts.js#L79
It possible you see an error in your terminal locally though since uploading doesn't require having the API secret but destroying does and you wouldn't have that. I just sent you details on discord.
Concerns
Capture on the controller looks good to me.
Extra state is fine too. There's no global state management tool in the app at the moment (no redux or similar or a global context and reducer) That's partly because in some ways apollo can kinda fill that gap, since it caches queries it's fine to just call fire those queries often, (you can even use apollo to do local state management, they have these local/client mutations etc that you can do, I'm not completely sold on it though). Plus everything part of state so far is either something that should be put in the db (so apollo kinda handles it) or it specific to a component so doesn't need full-blown state management. It's likely at some point that we might want/need proper state management, but I don't think it's something worth implementing when it's not needed yet, plus state management is one of the few things that RedWood doesn't have a strong opinion on, and I half expected that to change. If they come out with an officially recommended way of doing it I'm inclined to follow them from the perspective of keeping it as standard as possible as a way of making it easier for others to contribute. Sorry that was a long way of saying that adding the extra useState is 100% fine. If the toolbar starts getting too crowded we can make each button it's own small component instead, that's probably more the direction I'd go.
That's a good point. That flash is just the standard Redwood one, and this page is not the only place that it pushing down content is a little jaring, a quick fix, like you said
position: fixedwill work. I just addedfixed w-screen z-10to this line. Maybe at some point we'll change it to a more custom toast message. (I've had my eye on https://react-hot-toast.com/ for a while)changes
That's a lot of words, here's what I think should change.
That's all really it, you can tweak the flash if you want.
I'll have a bit of a look into capture on first save. 🤞
Woot thanks! ✨
Questions
Concerns
react-hot-toast, it looks crispy.Will comment in after I push the update.
I got capture on first save working.
104bc0ad70Happy for you to
cherry-pickthat commit or I can put up a PR after this one.Note that there are some formatting changes in that commit, the project automatically formats on save for me, it's not something I set up I just think that's how redwood is setup to work when it used in vscode, I was going to ask you to run
yarn rw lintto format your branch because I thought that was a way to get it to format without the auto-save, but I get an error when I do that 🤷 . I'm a couple of version behind latest-redwood so I should probably update it maybe that will fix that command. Anyway, formatting is not a big problem.Oh sick @Irev-Dev way to go! Do you mind doing that in a different PR? I don't feel confident with Git yet to do a
cherry-picklike that, you'll probably be way faster at that.My stuff is ready for your review when you're free. I made it so that if your part happens to not have a screenshot yet when you click Save Part Image button it saves right away without needing to click the second thing as well.
Thanks again @franknoirot.
The app is definitely a lot better because of this.