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

[SimpleUploadAdapter] Consider responsive images in server response #2840

Closed
oleq opened this issue Jul 17, 2019 · 4 comments · Fixed by ckeditor/ckeditor5-upload#98
Closed
Assignees
Labels
package:upload type:improvement This issue reports a possible enhancement of an existing feature.
Milestone

Comments

@oleq
Copy link
Member

oleq commented Jul 17, 2019

ATM the adapter allows only a single url in the response from the server. This makes implementing responsive images impossible using the adapter. Let's consider allowing multiple URLs to different image sizes in the server response.

@Reinmar
Copy link
Member

Reinmar commented Jul 17, 2019

Definitely 👍 . The EasyImage service supports responsive images already, so all the things are there in place also on CKE5 side. In fact, what will we have to change in SUA itself to enable that? Sounds like a micro change.

@oleq
Copy link
Member Author

oleq commented Jul 19, 2019

To keep things responsive images consistent among different upload adapters:

  1. Change the required server response format (docs, code) from:
    {
        url: 'http://example.com/images/image–default-size.png',
    }
    
    to
    { 
        urls: {
             default: 'http://example.com/images/image–default-size.png',
             '160': 'http://example.com/images/image–size-160.image.png',
             '500': 'http://example.com/images/image–size-500.image.png',
             '1000': 'http://example.com/images/image–size-1000.image.png',
             '1052': 'http://example.com/images/image–default-size.png'
        }
    }
    
  2. Change the following line https://github.com/ckeditor/ckeditor5-upload/blob/master/src/adapters/simpleuploadadapter.js#L179-L181 to resolve( response.urls );
  3. Update docs.

@Reinmar
Copy link
Member

Reinmar commented Jul 19, 2019

How do EI and CKF work? One of them uses responsive images, the other doesn't. Couldn't simple upload adapter support both cases?

@Reinmar
Copy link
Member

Reinmar commented Jul 19, 2019

OK, stupid question. You linked to the docs.

So, I'm fine with both options:

  • supporting both schemes (e.g. having such a normalisation: data.url ? { default: data.url } : data.urls)
  • or supporting just the latter.

@oleq oleq self-assigned this Aug 1, 2019
jodator referenced this issue in ckeditor/ckeditor5-upload Aug 1, 2019
Feature: Implemented the responsive image support in the `SimpleUploadAdapter`. Closes #97.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-upload Oct 9, 2019
@mlewand mlewand added this to the iteration 26 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:improvement This issue reports a possible enhancement of an existing feature. package:upload labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:upload type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants