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

When dropping an unsupported file onto a notebook entry, tell the user it isn't supported #7097

Closed
scottbell opened this issue Sep 28, 2023 · 5 comments · Fixed by #7115
Closed
Labels
type:enhancement verified Tested or intentionally closed
Milestone

Comments

@scottbell
Copy link
Contributor

scottbell commented Sep 28, 2023

Is your feature request related to a problem? Please describe.
When dropping an supported file onto a notebook entry (e.g., non-image or an image too big), tell the user it isn't supported - don't just silently fail.

Describe the solution you'd like
Perhaps a notification that the embed didn't work because it wasn't the right type of file (an image), or it was too big.

Note that for the issue of an image being too big, this does not affect the initial import of the file into the entry, but rather when the user is doing some action with the image afterwards (i.e., annotating). When the image is annotated and saved, only then do we see errors coming from the console, which stop the annotations from persisting.

@scottbell scottbell changed the title When dropping a non-image onto an entry, tell the user it isn't supported When dropping an unsupported file onto a notebook entry, tell the user it isn't supported Oct 4, 2023
@scottbell scottbell self-assigned this Oct 4, 2023
@scottbell
Copy link
Contributor Author

@scottbell to document how/why images are getting bigger post annotations. My current theory is when we annotate, we re-save the image much larger than the original.

@scottbell
Copy link
Contributor Author

@rukmini-bose found that an image that's 2MB in size goes to 4.9 MB after adding a circle, which points to the image library being a bit inefficient about encoding

@scottbell
Copy link
Contributor Author

I've added notifications & errors for:

  • Unknown files
  • Couch errors on initial save of image (e.g., too big)

I've also updated the README in the CouchDB plugin to describe how to up the documents limits. Basically:

  [couchdb]
  max_document_size = 4294967296 ; approx 4 GB
  
  [chttpd]
  max_http_request_size = 4294967296 ; approx 4 GB

as both the documents, and the HTTP server in front of CouchDB need to accept the new big limits.

I've also updated interaction with the entry to accept pasting of images, both in the text box, and if selecting the entry.

What I haven't done is intercept a CouchDB error when annotating an image that's become too big, as this happens during a mutation, which unfortunately isn't propagated all the way back. We could fix this by allowing mutate in ObjectAPI to return a Promise, e.g.:

mutate(domainObject, path, value) {
    this.#mutate(domainObject, path, value);

    if (this.isTransactionActive()) {
      return Promise.resolve(this.transaction.add(domainObject));
    } else {
      return this.save(domainObject);
    }
  }

and then catching it on the Notebook side, but this is touching some core Open MCT stuff, and would need @akhenry's blessing.

@scottbell
Copy link
Contributor Author

To test:

  1. Create a notebook
  2. Drop an unsupported file onto the notebook (e.g., a javascript file)
  3. Note the error
  4. Drop a really big image file onto the notebook (> 20 megabytes)
  5. Note the couch error
  6. Drop a normal size image onto the notebook (~6 megabytes)
  7. Annotate the image, click done
  8. Note the couch error waiting for you

Also, try copy/pasting images into both the entry text, and just selecting the entry. It should create embeddings for you.

@shefalijoshi
Copy link
Contributor

Verified fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement verified Tested or intentionally closed
Projects
None yet
4 participants