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

Mobile: Add image editor #6739

Conversation

personalizedrefrigerator
Copy link
Collaborator

Functionality discussion:

Summary

  • This PR adds:
    • an option to draw an image after clicking attach in the mobile app.
    • re-maps the open long-press menu item to edit drawn images (currently determined by images with mime: image/svg+xml).

To-do (for this PR)

  • Fix occasional NaN error while stroke smoothing
  • Add tests for loading/saving
  • Detect if an SVG wasn't made in Joplin and warn the user?
    • There can be some path-related data loss (the editor doesn't support some path commands), but otherwise the editor preserves unrecognized SVG tags. ...so perhaps this isn't necessary?

Math blocks and regions of inline math were missing their original
containing nodes.
@laurent22
Copy link
Owner

I'm surprised a custom image editor had to be developed, especially since it's simply rendered in a webview eventually. Wasn't it possible to use one of the existing JS image editors?

@personalizedrefrigerator
Copy link
Collaborator Author

personalizedrefrigerator commented Aug 12, 2022

Wasn't it possible to use one of the existing JS image editors?

I created a custom image editor because I wanted (among other things):

  1. An infinite zooming feature (not quite infinite in this implementation, but very, very large)
  2. Extensibility
  3. Primarily designed for touch/pen input but also able to work well with mouse/keyboard input
    • Ability to rotate the screen while zooming
    • A GUI that fits nicely on a phone/tablet screen
  4. Be maintained

(My primary reason for making a custom image editor was item 1 above — I didn't find an existing editor that provided this and I find it very, very nice for adding details to existing notes. I didn't find an existing app that does this.).

@laurent22
Copy link
Owner

The issue however is that a full JS image editor is a big project, and that will have to be maintained over the years. Whenever we add such project, for example the web clipper, or Joplin Server, we have to weight the pros and cons - is the added value really worth the extra work this is going to represent? For the clipper and server, I think it's worth it. For an image editor I'm not convinced.

I understand that you might not have found certain features in other libs, like the zoom, but not having this may be acceptable if it means we don't have to maintain another large project.

Basically such a decision should be made carefully. There should have been a discussion to look at existing libs, and see if we can use one of them, what would be the pros and cons of each of them, and whether it makes sense to create a custom editor. Or maybe there was one and I missed it?

Unfortunately you already went quite far with this but I still think this discussion should happen.

@personalizedrefrigerator
Copy link
Collaborator Author

personalizedrefrigerator commented Aug 12, 2022

The issue however is that a full JS image editor is a big project, and that will have to be maintained over the years. Whenever we add such project, for example the web clipper, or Joplin Server, we have to weight the pros and cons - is the added value really worth the extra work this is going to represent? For the clipper and server, I think it's worth it. For an image editor I'm not convinced.

I'm willing to maintain the image editor myself as a separate npm package.

Basically such a decision should be made carefully. There should have been a discussion to look at existing libs, and see if we can use one of them, what would be the pros and cons of each of them, and whether it makes sense to create a custom editor. Or maybe there was one and I missed it?

Unfortunately you already went quite far with this but I still think this discussion should happen.

Definitely! I've created a forum post for this: https://discourse.joplinapp.org/t/image-editor-project-is-maintaining-a-custom-image-editor-worth-it/26855

@personalizedrefrigerator
Copy link
Collaborator Author

I'm planning to use Fabric.js to manage objects/SVG import and export. Closing until this is done.

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

Successfully merging this pull request may close these issues.

2 participants