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

Update legacy theme styles to add responsiveness to featured image #7560

Merged
merged 16 commits into from
Jul 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 40 additions & 17 deletions .github/workflows/build-test-measure.yml
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,36 @@ jobs:
repo-token: '${{ secrets.GITHUB_TOKEN }}'
report-json: 'lint-js-report.json'

#-----------------------------------------------------------------------------------------------------------------------

normalize-composer:
name: 'Normalize composer.json'
needs: pre-run
if: needs.pre-run.outputs.changed-php-count > 0 || needs.pre-run.outputs.changed-gha-workflow-count > 0
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3

- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: '8.1'
coverage: none

- name: Get composer-normalize.phar
run: |
wget https://github.com/ergebnis/composer-normalize/releases/latest/download/composer-normalize.phar
chmod +x composer-normalize.phar

- name: Validate composer.json
run: composer --no-interaction validate --no-check-all

- name: Normalize composer.json
run: |
composer config --no-interaction --no-plugins allow-plugins.ergebnis/composer-normalize true
./composer-normalize.phar --dry-run

#-----------------------------------------------------------------------------------------------------------------------

lint-php:
Expand Down Expand Up @@ -189,18 +219,9 @@ jobs:
- name: Install Composer dependencies
run: composer install --prefer-dist --optimize-autoloader --no-progress --no-interaction

- name: Validate composer.json
run: composer --no-interaction validate --no-check-all

- name: Detect coding standard violations (PHPCS)
run: vendor/bin/phpcs -q --report=checkstyle --runtime-set ignore_errors_on_exit 1 --runtime-set ignore_warnings_on_exit 1 | cs2pr --graceful-warnings

- name: Normalize composer.json
run: |
composer config --no-interaction --no-plugins allow-plugins.ergebnis/composer-normalize true
composer require --no-interaction --dev ergebnis/composer-normalize --ignore-platform-reqs
composer --no-interaction normalize --dry-run

#-----------------------------------------------------------------------------------------------------------------------

static-analysis-php:
Expand Down Expand Up @@ -362,12 +383,13 @@ jobs:
mysql:
image: mariadb:latest
env:
MYSQL_ALLOW_EMPTY_PASSWORD: true
MYSQL_ROOT_PASSWORD:
MYSQL_DATABASE: wordpress_test
MARIADB_ALLOW_EMPTY_ROOT_PASSWORD: true
MARIADB_DATABASE: wordpress_test
MARIADB_MYSQL_LOCALHOST_USER: 1
MARIADB_MYSQL_LOCALHOST_GRANTS: USAGE
ports:
- 3306
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
options: --health-cmd="healthcheck.sh --su-mysql --connect --innodb_initialized" --health-interval=10s --health-timeout=5s --health-retries=3
continue-on-error: ${{ matrix.experimental == true }}
strategy:
fail-fast: false
Expand Down Expand Up @@ -606,12 +628,13 @@ jobs:
mysql:
image: mariadb:latest
env:
MYSQL_ALLOW_EMPTY_PASSWORD: true
MYSQL_ROOT_PASSWORD:
MYSQL_DATABASE: wordpress_test
MARIADB_ALLOW_EMPTY_ROOT_PASSWORD: true
MARIADB_DATABASE: wordpress_test
MARIADB_MYSQL_LOCALHOST_USER: 1
MARIADB_MYSQL_LOCALHOST_GRANTS: USAGE
ports:
- 3306
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
options: --health-cmd="healthcheck.sh --su-mysql --connect --innodb_initialized" --health-interval=10s --health-timeout=5s --health-retries=3
continue-on-error: ${{ matrix.experimental == true }}
strategy:
fail-fast: false
Expand Down
3 changes: 2 additions & 1 deletion bin/ci/install-wp-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ install_db() {
fi

# create database
mysqladmin create "$DB_NAME" --user="$DB_USER" --password="$DB_PASS""$EXTRA"
mariadb-admin create $DB_NAME --user="$DB_USER" --password="$DB_PASS"$EXTRA || \
mysqladmin create "$DB_NAME" --user="$DB_USER" --password="$DB_PASS"$EXTRA
}

install_wp
Expand Down
2 changes: 2 additions & 0 deletions includes/embeds/class-amp-core-block-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,7 @@ private function process_text_widgets( Document $dom ) {
foreach ( $dom->xpath->query( '//div[ @class = "textwidget" ]' ) as $text_widget ) {
// Restore the width/height attributes which were preserved in preserve_widget_text_element_dimensions.
foreach ( $dom->xpath->query( sprintf( './/*[ @%s or @%s ]', self::AMP_PRESERVED_WIDTH_ATTRIBUTE_NAME, self::AMP_PRESERVED_HEIGHT_ATTRIBUTE_NAME ), $text_widget ) as $element ) {
/** @var DOMElement $element */
if ( $element->hasAttribute( self::AMP_PRESERVED_WIDTH_ATTRIBUTE_NAME ) ) {
$element->setAttribute( Attribute::WIDTH, $element->getAttribute( self::AMP_PRESERVED_WIDTH_ATTRIBUTE_NAME ) );
$element->removeAttribute( self::AMP_PRESERVED_WIDTH_ATTRIBUTE_NAME );
Expand All @@ -586,6 +587,7 @@ private function process_text_widgets( Document $dom ) {
* responsive so this is built-in. Note also the style rule for .wp-video in amp-default.css.
*/
foreach ( $dom->xpath->query( './/div[ @class = "wp-video" and @style ]', $text_widget ) as $element ) {
/** @var DOMElement $element */
$element->removeAttribute( 'style' );
}
}
Expand Down
2 changes: 2 additions & 0 deletions includes/embeds/class-amp-tumblr-embed-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ public function sanitize_raw_embeds( Document $dom ) {
);
$overflow_element->textContent = __( 'See more', 'amp' );
$amp_element->appendChild( $overflow_element );

/** @var DOMElement $placeholder */
$placeholder->setAttribute( Attribute::PLACEHOLDER, '' );
$amp_element->appendChild( $placeholder );

Expand Down
1 change: 1 addition & 0 deletions includes/sanitizers/class-amp-style-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,7 @@ public function sanitize() {
}
} else {
foreach ( $styled_elements as $element ) {
/** @var DOMElement $element */
$attr = $element->getAttributeNode( Attribute::STYLE );
if ( $attr && preg_match( '/!\s*important/i', $attr->value ) ) {
ValidationExemption::mark_node_as_px_verified( $attr );
Expand Down
57 changes: 33 additions & 24 deletions src/ReaderThemeSupportFeatures.php
Original file line number Diff line number Diff line change
Expand Up @@ -579,40 +579,49 @@ public function get_relative_luminance_from_hex( $hex ) {
* @return bool False if `wp_get_global_settings()` not exists or theme.json not found, true otherwise.
*/
private function theme_has_theme_json() {
if ( function_exists( 'wp_theme_has_theme_json' ) ) {
return wp_theme_has_theme_json();
// @TODO: Uncomment this once `wp_theme_has_theme_json()` caching is fixed.
// if ( function_exists( 'wp_theme_has_theme_json' ) ) {
// return wp_theme_has_theme_json();
// }

static $theme_has_support = null;
static $prev_stylesheet_directory = null;
static $prev_template_directory = null;

$stylesheet_directory = get_stylesheet_directory();
$template_directory = get_template_directory();

// Make sure that the cached $theme_has_support value is reset when the theme changes.
if ( null !== $theme_has_support && (
$stylesheet_directory !== $prev_stylesheet_directory ||
$template_directory !== $prev_template_directory
) ) {
$theme_has_support = null;
}

static $theme_has_support = null;
$prev_stylesheet_directory = $stylesheet_directory;
$prev_template_directory = $template_directory;

if (
null !== $theme_has_support &&

/*
* Ignore static cache when `WP_DEBUG` is enabled. Why? To avoid interfering with
* the theme developer's workflow.
*
* @todo Replace `WP_DEBUG` once an "in development mode" check is available in Core.
*/
! ( defined( 'WP_DEBUG' ) && WP_DEBUG ) &&

/*
* Ignore cache when automated test suites are running. Why? To ensure
* the static cache is reset between each test.
*/
! ( defined( 'WP_RUN_CORE_TESTS' ) && WP_RUN_CORE_TESTS )
// Ignore static cache when the development mode is set to 'theme', to avoid interfering with
// the theme developer's workflow.
( function_exists( 'wp_get_development_mode' ) && wp_get_development_mode() !== 'theme' )
) {
return $theme_has_support;
}

// Does the theme have its own theme.json?
$theme_has_support = is_readable( get_stylesheet_directory() . '/theme.json' );

// Look up the parent if the child does not have a theme.json.
if ( ! $theme_has_support ) {
$theme_has_support = is_readable( get_template_directory() . '/theme.json' );
// This is the same as get_theme_file_path(), which isn't available in load-styles.php context.
if ( $stylesheet_directory !== $template_directory && file_exists( $stylesheet_directory . '/theme.json' ) ) {
$path = $stylesheet_directory . '/theme.json';
} else {
$path = $template_directory . '/theme.json';
}

/** This filter is documented in wp-includes/link-template.php */
$path = apply_filters( 'theme_file_path', $path, 'theme.json' );

$theme_has_support = file_exists( $path );

return $theme_has_support;
}

Expand Down
5 changes: 5 additions & 0 deletions templates/style.php
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,11 @@
.amp-wp-article-featured-image {
margin: 0 0 1em;
}
.amp-wp-article-featured-image img:not(amp-img) {
max-width: 100%;
height: auto;
margin: 0 auto;
}
.amp-wp-article-featured-image amp-img {
margin: 0 auto;
}
Expand Down
28 changes: 25 additions & 3 deletions tests/php/src/Admin/PolyfillsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ public function test_registration() {

// These should pass in WP < 5.6.
$this->assertTrue( wp_script_is( 'lodash', 'registered' ) );
$this->assertStringContainsString( '_.noConflict();', wp_scripts()->print_inline_script( 'lodash', 'after', false ) );
$this->assertStringContainsString( '_.noConflict();', self::get_inline_script( 'lodash', 'after' ) );

$this->assertTrue( wp_script_is( 'wp-api-fetch', 'registered' ) );
$this->assertStringContainsString( 'createRootURLMiddleware', wp_scripts()->print_inline_script( 'wp-api-fetch', 'after', false ) );
$this->assertStringContainsString( 'createNonceMiddleware', wp_scripts()->print_inline_script( 'wp-api-fetch', 'after', false ) );
$this->assertStringContainsString( 'createRootURLMiddleware', self::get_inline_script( 'wp-api-fetch', 'after' ) );
$this->assertStringContainsString( 'createNonceMiddleware', self::get_inline_script( 'wp-api-fetch', 'after' ) );

$this->assertTrue( wp_script_is( 'wp-hooks', 'registered' ) );
$this->assertTrue( wp_script_is( 'wp-i18n', 'registered' ) );
Expand All @@ -99,4 +99,26 @@ public function test_registration() {

$this->assertTrue( wp_style_is( 'wp-components', 'registered' ) );
}

/**
* Get inline script.
*
* @param string $handle Script handle.
* @param string $position Script position.
*
* @return string
*/
public static function get_inline_script( $handle, $position = 'after' ) {
if ( method_exists( wp_scripts(), 'get_inline_script_tag' ) ) {
$script = wp_scripts()->get_inline_script_tag( $handle, $position );

if ( ! $script ) {
return false;
}

return $script;
} else {
return wp_scripts()->print_inline_script( $handle, $position, false );
}
}
}
13 changes: 7 additions & 6 deletions tests/php/src/Editor/EditorSupportTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use AmpProject\AmpWP\Infrastructure\Service;
use AmpProject\AmpWP\Tests\Helpers\WithBlockEditorSupport;
use AmpProject\AmpWP\Tests\TestCase;
use AmpProject\AmpWP\Tests\Admin\PolyfillsTest;

/** @coversDefaultClass \AmpProject\AmpWP\Editor\EditorSupport */
final class EditorSupportTest extends TestCase {
Expand Down Expand Up @@ -71,15 +72,15 @@ public function test_supports_current_screen( $post_type_uses_block_editor, $pos
/** @covers ::maybe_show_notice() */
public function test_dont_show_notice_if_no_screen_defined() {
$this->instance->maybe_show_notice();
$this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) );
$this->assertFalse( PolyfillsTest::get_inline_script( 'wp-edit-post', 'after' ) );
}

/** @covers ::maybe_show_notice() */
public function test_dont_show_notice_for_unsupported_post_type() {
$this->setup_environment( true, false );

$this->instance->maybe_show_notice();
$this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) );
$this->assertFalse( PolyfillsTest::get_inline_script( 'wp-edit-post', 'after' ) );
}

/** @covers ::maybe_show_notice() */
Expand All @@ -92,11 +93,11 @@ public function test_show_notice_for_supported_post_type() {

$this->instance->maybe_show_notice();
if ( $this->instance->is_current_screen_block_editor_for_amp_enabled_post_type() ) {
$this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) );
$this->assertFalse( PolyfillsTest::get_inline_script( 'wp-edit-post', 'after' ) );
} else {
$this->assertStringContainsString(
'AMP functionality is not available',
wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false )
PolyfillsTest::get_inline_script( 'wp-edit-post', 'after' )
);
}
}
Expand All @@ -108,7 +109,7 @@ public function test_maybe_show_notice_for_unsupported_user() {

$this->instance->maybe_show_notice();

$this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) );
$this->assertFalse( PolyfillsTest::get_inline_script( 'wp-edit-post', 'after' ) );
}

/** @covers ::maybe_show_notice() */
Expand All @@ -125,7 +126,7 @@ public function test_maybe_show_notice_for_gutenberg_4_9() {
wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) );

$this->instance->maybe_show_notice();
$inline_script = wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false );
$inline_script = PolyfillsTest::get_inline_script( 'wp-edit-post', 'after' );
$this->assertStringContainsString( 'AMP functionality is not available', $inline_script );
}
}
7 changes: 3 additions & 4 deletions tests/php/src/ReaderThemeSupportFeaturesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -483,16 +483,15 @@ public function test_print_theme_support_styles_reader( $is_legacy ) {
$this->assertStringContainsString( '<style id="amp-wp-theme-support-editor-font-sizes">', $output );
$this->assertStringContainsString( 'font-size: clamp(', $output );
$this->assertStringContainsString( '+ ((', $output );
$this->assertStringContainsString( ':root .has-small-font-size { font-size: clamp(0.875rem, 0.875rem + ((1vw - 0.48rem) * 0.24), 1rem); }', $output );
$this->assertStringContainsString( ':root .has-small-font-size { font-size: clamp(0.875rem', $output );
}

// Assert spacing size custom properties.
if ( $this->call_private_method( $this->instance, 'theme_has_theme_json' ) && function_exists( 'wp_get_global_settings' ) ) {
$this->assertStringContainsString( '<style id="amp-wp-theme-support-spacing-sizes-custom-properties">', $output );
$this->assertStringContainsString( ':root {', $output );
$this->assertStringContainsString( '--wp--preset--spacing--30: clamp(1.5rem, 5vw, 2rem);', $output );
$this->assertStringContainsString( 'clamp(1.8rem, 1.8rem + ((1vw - 0.48rem) * 2.885), 3rem);', $output );
$this->assertStringContainsString( '--wp--preset--spacing--80: clamp(7rem, 14vw, 11rem);', $output );
$this->assertStringContainsString( '--wp--preset--spacing--30:', $output );
$this->assertStringContainsString( '--wp--preset--spacing--80:', $output );
$this->assertStringContainsString( '}', $output );
$this->assertStringContainsString( '</style>', $output );
}
Expand Down
3 changes: 3 additions & 0 deletions tests/php/test-amp-gallery-embed-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,9 @@ public function test__conversion( $source, $expected, $use_legacy_mode = false )
// Remove lazy loading attribute.
$content = preg_replace( '/\s+loading="lazy"/', '', $content );

// Remove fetchpriority attribute.
$content = preg_replace( '/\s+fetchpriority="high"/', '', $content );

$this->assertEquals(
$this->normalize( $expected ),
$this->normalize( $content )
Expand Down