ckeditor / ckeditor4-sdk

A set of software development tools for CKEditor 4 along with samples.
Other
18 stars 21 forks source link

Easy Image demo issues #257

Open m-kr opened 6 years ago

m-kr commented 6 years ago

While I'm porting demo to the website process, I've noticed some issues here, IDK whether this is an only wrong interpretation or a real problem.

Possibility to display 100% optimization.

https://github.com/ckeditor/ckeditor-sdk/blob/master/samples/assets/easyimage/optimizations.js#L47

return formatSize( optimized ) + ' (' + Math.ceil( 100 - ( optimized * 100 / original ) ) + '%)';

This allows to show 100% optimization which is true only if you reduce size to 0 B 😉

Take original image size from the cloud response.

https://github.com/ckeditor/ckeditor-sdk/blob/master/samples/assets/easyimage/optimizations.js#L119

original.size = data[ 'default' ].size;

Why don't we take the total value from the evt.data.loader? If there is any 502 or 500 error from the Cloud then all calculations fail, which seems to happen very often.

Many 502 errors from the CS

When uploaded image is quite heavy then very often some of the optimized version has error status 502 received from CS. Then displaying optimization info is not possible. @czerwonkabartosz is checking now what is the reason. I'll paste some info when we find out where is the bug.

m-kr commented 6 years ago

screen shot 2018-06-11 at 12 45 41

I wonder also whether not displaying optimization for phone device is correct, in a case when the lowest value is a bit bigger than iPhone's or Samsung's. For those devices image src will be the same as on a Tablet, but IMHO it's OK to show all of them. We can reduce the chance of confusing users, whose are not much familiar with the responsive images.

wwalc commented 6 years ago

We're showing wrong results. On Full HD 1640x923 should not be used because the image is too small for such screen.