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

Handle embedded images added to a drawing #199

Open
Tracked by #211
bric3 opened this issue Jan 12, 2024 · 17 comments · May be fixed by #200
Open
Tracked by #211

Handle embedded images added to a drawing #199

bric3 opened this issue Jan 12, 2024 · 17 comments · May be fixed by #200
Labels
enhancement New feature or request
Milestone

Comments

@bric3
Copy link
Owner

bric3 commented Jan 12, 2024

  1. Copy image to excalidraw
  2. Close drawing
  3. Re-open

The drawing have the placeholder for the image, but the image was not saved.

[!INFO]
The issue is the lack of APIs to load binary files in the excalidraw js library (see #200).

@bric3 bric3 added this to the 0.4.0 milestone Jan 12, 2024
@bric3 bric3 mentioned this issue Jan 12, 2024
9 tasks
bric3 added a commit that referenced this issue Jan 12, 2024
@bric3 bric3 linked a pull request Jan 12, 2024 that will close this issue
2 tasks
@bric3 bric3 modified the milestones: 0.4.0, 0.4.1 Jan 27, 2024
@ixqbar
Copy link

ixqbar commented Apr 16, 2024

+1

@bric3
Copy link
Owner Author

bric3 commented Apr 22, 2024

Feel free to contribute to speed things up.

@lionelhorn
Copy link

@bric3

I tried to have a look at the issue.
My approch was to get the initialData and only then render the

 <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

lionelhorn pushed a commit to lionelhorn/excalidraw-jetbrains-plugin that referenced this issue Jul 21, 2024
lionelhorn added a commit to lionelhorn/excalidraw-jetbrains-plugin that referenced this issue Jul 21, 2024
- 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
@lionelhorn
Copy link

Changed too much stuff in my fork to be able to do a small PR. But I linked my version above.
I would be glad to help to get this issue fixed. Let me know.

@bric3
Copy link
Owner Author

bric3 commented Jul 23, 2024

@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.
Does an iframe is necessary ?
By the way I like how you split the files, it feels much better and cleaner that way.

By the way thank you very much for helping me fixing this.

@lionelhorn
Copy link

@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.
When I made a change to the html UI, the iframe just updated with the new code.

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.

@lionelhorn
Copy link

I tried to do a simpler rewrite by not introducing any additinal deps (no vite, no tailwind, no mobx).
The only thing I didn't manage to do was a clean api with only react state management (useState)

As there is a need for the Excalidraw component to re-render based on viewMode, theme, etc..

@bric3
Copy link
Owner Author

bric3 commented Jul 26, 2024

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.

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 ?

I tried to do a simpler rewrite by not introducing any additinal deps (no vite, no tailwind, no mobx).
The only thing I didn't manage to do was a clean api with only react state management (useState)

Yeah that feels like a smaller step.

I'll think of how to hot reload things. Are you using directly yarn or other nodejs based executable to build instead of gradle tasks ?

@lionelhorn
Copy link

lionelhorn commented Jul 29, 2024

I just googled HMR, is it Hot Module Replacement ?

Yes

I'll think of how to hot reload things. Are you using directly yarn or other nodejs based executable to build instead of gradle tasks ?

When I use vite for a frontend app like here. I just use the build tooling included in vite started via vite build

@bric3
Copy link
Owner Author

bric3 commented Aug 3, 2024

@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.

@bric3
Copy link
Owner Author

bric3 commented Aug 3, 2024

@lionelhorn I pushed a preliminary support for loading the web-app from a different path. At this time when the sandboxed ID is run (Run Excalidraw Plugin), or via ./gradlew :plugin:runIde it teels the plugin to load files from

excalidraw-assets
├── build
│   ├── assets
│   ├── react-build <2>
│   └── ...
└── ...

Where the assets directory contains Excalidraw JS and fonts and react-build is where the app build is located.

In order to generate those directories you can use ./gradlew :excalidraw-assets:assembleFrontend (which can be run from IntelliJ, Gradle toolwindow then select the right task in excalidraw-assets). To use the new web-app, just close the current editor in the sandbox and re-open one, it will load the updates files.
At this time this kinda tied to Gradle, I may improve the situation as I learn more.

More details in this commit : 2018b71

@lionelhorn
Copy link

Thanks @bric3 for the update. I'll have a look at the commit.

@bric3
Copy link
Owner Author

bric3 commented Aug 27, 2024

@lionelhorn FYI I'm looking at your changes and try to integrate them on the latest commit on master.

image

See #200

bric3 added a commit that referenced this issue Aug 27, 2024
@lionelhorn
Copy link

@bric3 Good news! Were you able to get it working on your end?

@bric3
Copy link
Owner Author

bric3 commented Aug 27, 2024

Yes :)

@bric3 bric3 changed the title Image added to a drawing are not properly saved Handle embedded images added to a drawing Aug 27, 2024
@bric3 bric3 added the enhancement New feature or request label Aug 27, 2024
@bric3
Copy link
Owner Author

bric3 commented Aug 27, 2024

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.

@bric3
Copy link
Owner Author

bric3 commented Sep 2, 2024

There's still a few issues, I'm not sure how to address this at this time.

  • TypeError: window.cefQuery is not a function
2024-09-02 09:40:31,435 [   9829]   WARN - #c.i.s.ComponentManagerImpl - com.intellij.psi.codeStyle.CodeStyleSettingsCustomizable initializer requests com.intellij.util.LocaleSensitiveApplicationCacheService instance
2024-09-02 09:40:33,574 [  11968]   WARN - #com.github.bric3.excalidraw.editor.ExcalidrawWebViewController - repro.excalidraw: runJS: controller is disposed: saveAsCoroutines
2024-09-02 09:40:33,574 [  11968]   WARN - #com.github.bric3.excalidraw.editor.ExcalidrawWebViewController - repro.excalidraw: runJS: controller is disposed: saveAsCoroutines
2024-09-02 09:40:33,906 [  12300] SEVERE - #com.github.bric3.excalidraw.editor.ExcalidrawWebViewController - hello.excalidraw: [LOGSEVERITY_ERROR][https://excalidraw-jetbrains-plugin/static/js/main.0e5247f0.js:43]:
Uncaught TypeError: window.cefQuery is not a function
java.lang.Throwable: hello.excalidraw: [LOGSEVERITY_ERROR][https://excalidraw-jetbrains-plugin/static/js/main.0e5247f0.js:43]:
Uncaught TypeError: window.cefQuery is not a function
	at com.intellij.openapi.diagnostic.Logger.error(Logger.java:376)
	at com.github.bric3.excalidraw.editor.ExcalidrawWebViewController$initJcefPanel$5.onConsoleMessage(ExcalidrawWebViewController.kt:219)
	at com.intellij.ui.jcef.JBCefClient$3.lambda$onConsoleMessage$4(JBCefClient.java:301)
	at com.intellij.ui.jcef.JBCefClient$HandlerSupport.lambda$handle$0(JBCefClient.java:733)
	at java.base/java.lang.Iterable.forEach(Iterable.java:75)
	at java.base/java.util.Collections$SynchronizedCollection.forEach(Collections.java:2131)
	at com.intellij.ui.jcef.JBCefClient$HandlerSupport.handle(JBCefClient.java:733)
	at com.intellij.ui.jcef.JBCefClient$HandlerSupport.handleBoolean(JBCefClient.java:738)
	at com.intellij.ui.jcef.JBCefClient$3.onConsoleMessage(JBCefClient.java:300)
	at jcef/org.cef.CefClient.onConsoleMessage(CefClient.java:354)
...
  • Slow operations on Event Dispatch Thread (maybe unrelated to this change)
2024-09-02 09:41:09,502 [  47896] SEVERE - #c.i.u.SlowOperations - Slow operations are prohibited on EDT. See SlowOperations.assertSlowOperationsAreAllowed javadoc.
java.lang.Throwable: Slow operations are prohibited on EDT. See SlowOperations.assertSlowOperationsAreAllowed javadoc.
	at com.intellij.openapi.diagnostic.Logger.error(Logger.java:376)
	at com.intellij.util.SlowOperations.assertSlowOperationsAreAllowed(SlowOperations.java:101)
	at com.intellij.workspaceModel.core.fileIndex.impl.WorkspaceFileIndexDataImpl.ensureIsUpToDate(WorkspaceFileIndexDataImpl.kt:130)
	at com.intellij.workspaceModel.core.fileIndex.impl.WorkspaceFileIndexDataImpl.getFileInfo(WorkspaceFileIndexDataImpl.kt:75)
	at com.intellij.workspaceModel.core.fileIndex.impl.WorkspaceFileIndexImpl.getFileInfo(WorkspaceFileIndexImpl.kt:247)
	at com.intellij.workspaceModel.core.fileIndex.impl.WorkspaceFileIndexImpl.findFileSet(WorkspaceFileIndexImpl.kt:203)
	at com.intellij.workspaceModel.core.fileIndex.impl.WorkspaceFileIndexImpl.isInContent(WorkspaceFileIndexImpl.kt:69)
	at com.intellij.openapi.roots.impl.ProjectFileIndexImpl.isInContent(ProjectFileIndexImpl.java:206)
	at com.intellij.openapi.fileEditor.impl.NonProjectFileWritingAccessProvider.isProjectFile(NonProjectFileWritingAccessProvider.java:136)
	at com.intellij.openapi.fileEditor.impl.NonProjectFileWritingAccessProvider.isWriteAccessAllowed(NonProjectFileWritingAccessProvider.java:122)
	at com.github.bric3.excalidraw.editor.ExcalidrawEditorProvider$createEditorAsync$1.build(ExcalidrawEditorProvider.kt:20)
...
  • Uncaught (in promise) Error: Excalidraw api not defined.
2024-09-02 09:42:07,651 [ 106045] SEVERE - #com.github.bric3.excalidraw.editor.ExcalidrawWebViewController - with-embedded-pic-2024-01-09-1133.excalidraw.svg: [LOGSEVERITY_ERROR][https://excalidraw-jetbrains-plugin/static/js/main.0e5247f0.js:43]:
Uncaught (in promise) Error: Excalidraw api not defined.
java.lang.Throwable: with-embedded-pic-2024-01-09-1133.excalidraw.svg: [LOGSEVERITY_ERROR][https://excalidraw-jetbrains-plugin/static/js/main.0e5247f0.js:43]:
Uncaught (in promise) Error: Excalidraw api not defined.
	at com.intellij.openapi.diagnostic.Logger.error(Logger.java:376)
	at com.github.bric3.excalidraw.editor.ExcalidrawWebViewController$initJcefPanel$5.onConsoleMessage(ExcalidrawWebViewController.kt:219)
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants