Handles custom tab sizes for gist embeds and shortcodes#13807
Handles custom tab sizes for gist embeds and shortcodes#13807
Conversation
|
Caution: This PR has changes that must be merged to WordPress.com |
|
|
||
| // Set the tab size, allowing attributes to override the query string. | ||
| $tab_size = $gist_info['ts']; | ||
| if ( ! empty( $atts['ts'] ) ) { |
There was a problem hiding this comment.
I decided to allow the attributes to override the query string to allow for theme filtering via shortcode_atts_{$shortcode} and similar.
There was a problem hiding this comment.
Should we add some default attributes in shortcode_atts at the top of github_gist_shortcode to allow for this to happen?
There was a problem hiding this comment.
The code already sets the defaults in jetpack_gist_get_shortcode_id. Do you feel that it needs to have that set additionally?
There was a problem hiding this comment.
We do indeed have defaults but we do not have the shortcode_atts function that would make the shortcode_atts_gist filter you mentioned above available, iirc. Maybe we could add that function here, to make the filter available?
Something like this?
jetpack/modules/shortcodes/recipe.php
Line 175 in 89a9af9
That's not necessarily a blocker here. 👍
| // inline style to prevent the bottom margin to the embed that themes like TwentyTen, et al., add to tables. | ||
| $return = '<style>.gist table { margin-bottom: 0; }</style><div class="gist-oembed" data-gist="' . esc_attr( $id ) . '"></div>'; | ||
| $return = sprintf( | ||
| '<style>.gist table { margin-bottom: 0; }</style><div class="gist-oembed" data-gist="%1$s" data-ts="%2$d"></div>', |
There was a problem hiding this comment.
Noting that we embed this multiple times. May be a better way to do this.
This is an automated check which relies on |
jeherve
left a comment
There was a problem hiding this comment.
What do you think about adding a new Unit Test to cover this use-case to our existing Gist tests here:
tests/php/modules/shortcodes/test-class.gist.php
|
ChaosExAnima, Your synced wpcom patch D34348-code has been updated. |
Resolves bugs with older tests that were failing with tab spacing code.
Test had extraneous whitespace, which let to WP parsing `/]` as closing of tag. Changed method of slash trimming to use `trim` and fixed test.
|
ChaosExAnima, Your synced wpcom patch D34348-code has been updated. |
|
|
||
| // Set the tab size, allowing attributes to override the query string. | ||
| $tab_size = $gist_info['ts']; | ||
| if ( ! empty( $atts['ts'] ) ) { |
There was a problem hiding this comment.
We do indeed have defaults but we do not have the shortcode_atts function that would make the shortcode_atts_gist filter you mentioned above available, iirc. Maybe we could add that function here, to make the filter available?
Something like this?
jetpack/modules/shortcodes/recipe.php
Line 175 in 89a9af9
That's not necessarily a blocker here. 👍
|
@jeherve - Did a bit of tinkering, and it looks like adding |
|
That works for me! |
* 7.9: Changelog * Update version number * Update stable tag and tested up to * Changelog: add #13530 * changelog: add #13578 * Changelog: add #13598 * Changelog: add entry for numerous block preview changes * Changelog: add #13599 * changelog: add #13541 * Changelog: add #13542 * Changelog: add #13331 * Changelog: add #13558 * Changelog: add #13409 * Changelog: add #13582 * Changelog: add #13600 * Changelog: add #13601 * Changelog: add #13595 * Changelog: add #12695 * Changelog: add #13009 * Changelog: add #13649 * Changelog: add #13450 * Changelog: add #13507 * Changelog: add #13658 * Changelog: add #13687 * changelog: add #13683 * Changelog: add #9323 * Changelog: add #13681 * Fix typos in readme * Add link to WordPress Beta Tester plugin * Changelog: add #13630 * Changelog: add #13695 * Changelog: add #13659 * Changelog: add #13716 * Changelog: add #13664 * Changelog: add #13682 * Changelog: add #13362 * Changelog: add #13563 * Add testing list for #13563 * Changelog: add #13735 * Changelog: add #13752 * Changelog: add #13624 * Changelog: add #13756 * Changelog: add #13745 * Changelog: add #13728 * Changelog: add #13779 * Changelog: add #13699 * Changelog: add #13804 * Changelog: add #13761 * Changelog: add #13637 * Changelog: add #13517 * Changelog: add #13521 * Changelog: add #13729 * Testing list: add testing instructions for #13729 * Changelog: add sync changes * Changelog: add #13807 * Changelog: add #13654 * Changelog: add #13795 * Changelog: add #13801 * Changelog: add #13818 * Changelog: add #13725 * Changelog: add #13831 * Changelog: add #13516 * Testing list: add Twenty Twenty instructions * Changelog: add #13799 * Changelog: add #13805 * Changelog: add #13688 * Changelog: add #13830
Fixes #10534
Changes proposed in this Pull Request:
?ts=query param.tsattribute to shortcodes.Known limitation: Currently, AMP does not have a way to change tab sizing. This is a limitation of the AMP
amp-gistelement.Is this a new feature or does it add/remove features to an existing part of Jetpack?
Testing instructions:
?ts=4to the gist URL.ts=4in it.Screenshot:
Proposed changelog entry for your changes: