-
Notifications
You must be signed in to change notification settings - Fork 11
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
Handle embedded images added to a drawing #199
Comments
+1 |
Feel free to contribute to speed things up. |
I tried to have a look at the issue. <Excalidraw /> component. Doing it like that, it seems there is no need to use the api to updateScene or anything. 2024-07-22.00-22-11.mp4 |
- POC status - bric3#199 - Added react vite app to get HMR on the app in the webview - Proxy messages from cef frame to iframe hosting vite app
Changed too much stuff in my fork to be able to do a small PR. But I linked my version above. |
@lionelhorn, I'm going on holiday soon. So I don't know when I will be able to have good look at your work. Also as you may have guessed, I'm far from a web-developer. I have a very shallow understanding of React itself, even Typescript. So when I see the proposition is to use Vite or tailwind, whose I know nothing but the name, I'm lost. I'm not against those two, but still I need to digest that. So maybe you could also help me by detailing the thought process why it's needed, etc. So I can have a better understanding of the stakes and the overall change. By the way thank you very much for helping me fixing this. |
@bric3 Many of the deps I used are not necessary but are what I'm familiar to and I used them to get something running quickly. My first issue (that may come from my lack of knowledge of the jetbrains plugin dev lifecycle & gradle) was that, in development mode, I needed to close the sandbox ide window and rerun the "run plugin" task just to see changes in the html and js assets. Which lead to a poor DX. That's why I added vite. For its HMR capabilities in dev. Tailwind is not necessary at all. Juste added it to style a little overlay debug div to display inner state on page (separate from the native jetbrains UI) Mobx is used for state management replacing react useState. Again not necessary. Just what was easier for me to get something cleaner code wise than passing the setters of useState to the bridge. Iframe probably not necessary either. Just let me reuse the gradle task and assets bundling from your existing excalidraw-assets gradle build as I'm not familiar enough with it to modify it. |
I tried to do a simpler rewrite by not introducing any additinal deps (no vite, no tailwind, no mobx). As there is a need for the Excalidraw component to re-render based on viewMode, theme, etc.. |
Yeah, I haven't quite got around that yet. It probably needs custom development mode that looks for a different source location. I just googled HMR, is it Hot Module Replacement ?
Yeah that feels like a smaller step. I'll think of how to hot reload things. Are you using directly |
Yes
When I use vite for a frontend app like here. I just use the build tooling included in vite started via |
@lionelhorn I have merged some work related to IntelliJ Plugin building (necessary when targeting 2024.2). I will try to find some time to improve the hot reload of the web-app without restarting the runned IJ. |
@lionelhorn I pushed a preliminary support for loading the web-app from a different path. At this time when the sandboxed ID is run (
Where the In order to generate those directories you can use More details in this commit : 2018b71 |
Thanks @bric3 for the update. I'll have a look at the commit. |
@lionelhorn FYI I'm looking at your changes and try to integrate them on the latest commit on master. See #200 |
@bric3 Good news! Were you able to get it working on your end? |
Yes :) |
I'd like to test a bit more before merging your work. Also, @lionelhorn maybe you could re-review the change, if you feel something is incorrect or could be improved on this code. I get the most of this change but I may have not captured everything and as such left some bug when porting your great changes. I'm really happy over how the code is now broken down to smaller pieces. |
There's still a few issues, I'm not sure how to address this at this time.
|
The drawing have the placeholder for the image, but the image was not saved.
The text was updated successfully, but these errors were encountered: