Generate embed codes through DocumentCloud's oEmbed endpoint #21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Fetch embed codes from the new oEmbed service instead of creating them manually (but leave existing functions in place until we're sure they can be discarded)
* Support both `maxwidth/maxheight` and `height/width` shortcode attrs, but only sends `max*` to the provider
* Nominally backwards-compatible with existing shortcodes and settings (should be tested)
* Piggy-backs on WordPress's internal oembed caching; to disable, change `add_shortcode('documentcloud', array(&$this, 'handle_dc_shortcode_with_caching'));` to `add_shortcode('documentcloud', array(&$this, 'handle_dc_shortcode'));`
Affects #16
Closes #14
* Create class constants for regularly-modifiable settings (mostly for testing) * Consolidate cached and non-cached shortcode call and just toggle where it mattesr Affects #16
I previously overcomplicated the height/width calculations out of deference to WordPress's internal height/width calculator. Realized the plugin already set its own defaults and thus I could simplify things by starting with that, and maintain better backwards-compatibility too. * Remove complicated height/width calculations * Make precedence order clear Affects #16
And make it supreme above all other width declarations to be consistent with previous plugin behavior. Affects #16
* Adjust `save()` as minimally as necessary to support new function names and the dual-`width/maxwidth` presence * Add note re: need for new post_metadata key * Add note re: functions we should be able to remove once tested Affects #16
Affects #16
Was committed accidentally. Not ready to be the default within WP. Affects #16
The readme.txt for WordPress looks ugly here at GitHub. Copy relevant info over.
WP likes upgrade notices. Here's an upgrade notice. Since you like upgrade notices.
Accidentally converted to HTTP
Per discussion and consensus among @eyeseast, @aschweigert, @knowtheory, @anthonydb and @reefdog. Affects #14
`$SITEURL` seems to behave inconsistently, so just buffaloing a solution by checking for trailing slash and adding it if it doesn't exist.
When there's no shortcode or user-defined default, use the theme's defaults for embed sizes instead of defining our own final defaults. Closes #19
Trying to balance user intent: now that we're responsive by default, it stands to reason that a user indicating a width directly on a shortcode actually wants that width, so we should disable responsiveness unless they have actually ALSO indicated they want responsiveness, in which case they are confused and we'll go with responsive. Affects #14
This started life as a task to include note ID in the post metadata when it was keyed off a note, since the document slug is the same on notes and documents, and thus two resources from the same doc could cause a ID collision. Then I realized that we already only save a single piece of metadata per post anyway (see how `update_post_meta()` is outside the `foreach()`?) and so this was unnecessary. But I'd already done the work to abstract out `parse_dc_url()` and clean up some things, so. Affects #22
|
We're happy enough with this to go ahead and pull it into master so we can tag and submit to WordPress plugin directory today. There are still a few issues (like the preexisting #23) but we can fix those as point releases. Unless @eyeseast or @aschweigert want us to hold off for a bit longer, I'd like to merge this in later this afternoon. |
Confirmed compatibility with 3.2 and 4.2.1.
Toolbar button has been broken for a while, since at least 3.9.1, because of an update to the version of TinyMCE included in WP. It needs some signifiant fixing, and could stand to have it fleshed out with additional attribute options anyway. Disabling until we have time for the full solution. Affects #23
Docs and notes have the same document slug, which resulted in the same postmeta key. When you had a wide document followed by a normal note, this resulted in the postmeta not properly reporting a `wide_asset`. Append note ID to its meta key so it can be distinguished from docs. Affects #22
reefdog
added a commit
that referenced
this pull request
Apr 29, 2015
Generate embed codes through DocumentCloud's oEmbed endpoint
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a proposal to replace Navis-DocumentCloud's existing embed code construction by deferring to DocumentCloud's oEmbed endpoint.
This release would include deploying a new plugin named Wordpress-DocumentCloud to supersede Navis-DocumentCloud.