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

make oembed configurable for additional oembed parameters #346

Conversation

reflexxion
Copy link
Contributor

Many OEmbed providers allow additional query parameters on their oembed services.
So on e.g. instagram there is a parameter for hiding the caption.

hidecaption

With this PR it is easy to configure that option from the outside:

        $embed = new Embed();

        $result = $embed->get('https://www.instagram.com/p/B_C0wheCa4V/');
        $result->getOEmbed()->setQueryParameters(['hidecaption' => true]);
        $code = $result->code;

@reflexxion reflexxion force-pushed the feature/configurable-oembed branch 2 times, most recently from 5507f90 to e8f6400 Compare April 17, 2020 14:20
@oscarotero
Copy link
Owner

Hi.
Thanks for your contribution. It's a nice feature, but this library have already a way to add settings to the detectors, as you can see here: https://github.com/oscarotero/Embed#settings

So, I'd rather something like this:

$embed = new Embed();

$result = $embed->get('https://www.instagram.com/p/B_C0wheCa4V/');
$result->setSettings([
    'oembed:query_parameters' => ['hidecaption' => true]
]);

$code = $result->code;

In this way, you can configure the settings of all detectors in a single place.

@reflexxion reflexxion force-pushed the feature/configurable-oembed branch 2 times, most recently from 9c9254b to 7fe563e Compare April 17, 2020 15:21
@reflexxion
Copy link
Contributor Author

Hey @oscarotero ,

oh sure.
I've changed it as proposed by you.

@oscarotero oscarotero merged commit a8fceaa into oscarotero:master Apr 17, 2020
@oscarotero
Copy link
Owner

Great. Thanks!

@reflexxion reflexxion deleted the feature/configurable-oembed branch April 17, 2020 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants