syndesisio / syndesis-rest

The API for Syndesis - a flexible, customizable, cloud-hosted platform that provides core integration capabilities as a service. It leverages Red Hat's existing product architecture using OpenShift Online/Dedicated and Fuse Integration Services.
https://syndesis-staging.b6ff.rh-idev.openshiftapps.com/api/v1/
Apache License 2.0
6 stars 17 forks source link

Add icons to the database #293

Closed KurtStam closed 7 years ago

KurtStam commented 7 years ago

Embed the icons are base64 in the returned JSON.

https://issues.jboss.org/browse/IPAAS-295

jimmidyson commented 7 years ago

Embedding the base64 version image in the JSON just bloats the response IMO and affects stuff like caching, which we should be looking to utilise for static assets as much as possible. I'd prefer to serve images from a separate endpoint and the JSON instead contain the link to the image.

Thoughts @gashcrumb, @kahboom?

jimmidyson commented 7 years ago

I am happy for this in two phases though, embedded in JSON for now and introduce a proper image service later, if doing the image service will take too long for now.

gashcrumb commented 7 years ago

I think it'd be just as easy to host the image files in the console and just use CSS classes from the json response. Definitely inlining images is error-prone, plus there's a size limit that you have to worry about, think it's 4k but it might be different cross-browser.

KurtStam commented 7 years ago

We just discussed on IRC with @kahboom, I'm happy to do whatever. Whatever is easier..

kahboom commented 7 years ago

So, there are several things we can do:

  1. Return PNG as JSON (must be encoded in Base64). Probably the most RESTful way of doing things, but as @jimmidyson pointed out, could get a bit bloaty. In our case, it's a small set of images that are maybe 100x100 pixels each and re-used frequently, but if we support uploading larger SVGs or PNGs that then need to be scaled, stored, etc. we should absolutely look into a dedicated service for this alone or it will become bloated. At that point, the API would just return the absolute path to the image within the connection or integration object and we'd reference that in the UI (see 4 below).
  2. Send PNG as multipart/form data. Not as easy to work with, but follows the HTTP spec and mimetypes the way they were intended. This is probably the best if we will definitely eventually support uploading SVGs and PNGs.
  3. Keep in the UI per @gashcrumb 's suggestion, and return a class name from the API (or use a unique identifier to reference them, like the name??). I'd normally gag and say 'no way', but again, these are a small set of icons and we're thinking short-term, specific solutions, so it's not too bad and probably the easiest to implement short term.
  4. Similar to the first option, except without a service, we can just have a public endpoint for images that whitelists the UI hostname, and the API would return the integration or connection as normal with an icon property as it does now, except this would be an absolute path to the public endpoint that loads the image directly. This requires more work in the API and overworks it for larger files (again, we'd need a dedicated service), but works just as well for now.
kahboom commented 7 years ago

cc @chirino

chirino commented 7 years ago

For now lets do the fastest thing possible.. so I'd got with @gashcrumb's suggestion and take this on as technical debt.

jimmidyson commented 7 years ago

Simples.

kahboom commented 7 years ago

UPDATE: We decided to go with option 3, with the exception of the class names. Was just discussing with @gashcrumb, we can just use the name or whatever property from the connector, it should map 1:1 for all common services. For now, while we still don't have users inputting the name of the images, etc.

My point is, we won't need to use the label property at all.. we can reference the connection's connector or integration's connection connector (if it's accessible). Whatever property defines the name of the service (e.g. twitter). It might need sanitization in the UI but would be even faster than labels.

In the HTML we'd do something like /icons/{{ connection.connector.name }}.connection.png and similar for integrations. The .connection.png would be to distinguish the image types, since there is apparently a different one for the Integration List and one for the Connection List, etc.

In other words, @KurtStam you don't have to do anything, for now, I think.. as we'll ignore the label property for now until we begin to accept user input where the name may vary.

Please keep this issue open for now until works out okay. Hoping to get this in today; need to re-scale the images to size so that the API and UI don't have to do this on the fly. Considering providing black&white versions for disabled states (need to consult UXD first) as well.

kahboom commented 7 years ago

cc @dongniwang @sjcox-rh - Please read above and consult about black&white icons for disabled states, or if we should do this on demand in the UI (preferably not). Also, I'll first be testing with 40x40 for the integration list, smaller for connection list. Let me know what you think.