Closed (fixed)
Project:
Media entity Instagram
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
21 May 2017 at 09:11 UTC
Updated:
11 Oct 2017 at 12:29 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
marcoscanoFirst attempt.
The simpletests this module was using were deliberately removed, once they depended on a trait that does not exist anymore in media core. Because of this, this issue should not get merged before #2880144: Port simple tests to browser-based tests.
Comment #3
marcoscanoSpotted some minor CS issues.
Comment #4
marcoscanoAnd now I realized that one of the attributes defined by
MediaSourceBase::getMetadata()was missing in our plugin implementation.Comment #5
marcoscanoComment #6
aspilicious commentedThis should be added in a new branch.
The old branch can be deprecated when 8.5/8.6 gets in.
Comment #7
marcoscanoYes, all ports to the new API should land in a new branch. There isn't one open for this project yet though, so I just created the patch against the current -dev, we should be able to bump it to 2.x as soon as it is created.
Comment #8
marcoscanoRe-rolling and minor tweaks.
Comment #9
chr.fritschNice work Marcos,
It works pretty good. Just one thing i discovered is that the thumbnail is not generated.
And maybe we should ship some default configuration, like we do in core now.
Comment #10
bkosborneWhy not just drupal:media (>= 8.4.0)
See media.install for their example. There is another if statement in there that ensures we don't overwrite an icon that may already exist.
does link make sense here?
Comment #11
marcoscanoThis addresses almost all feedback from #9 and #10.
Re: #10.3:
We had link earlier, so in the port I just kept what we had in the previous version.
@chr.fritsch Re: the thumbnail generation, I see that the local icon is being used. Do you mean to fetch a remote thumbnail instead? Does this work with the 1.x branch?
Comment #12
marcoscanoThisi should fix the remote thumbnail generation.
Comment #13
marcoscanoAfter discussion in the sprint at DrupalCon, it seems to be best to have no default configuration at all shipped with the contrib providers.
Comment #15
chr.fritschThank you everyone