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

Fix reveal background #8385

Draft
wants to merge 9 commits into
base: trunk
Choose a base branch
from
Draft

Fix reveal background #8385

wants to merge 9 commits into from

Conversation

renatho
Copy link
Contributor

@renatho renatho commented Nov 7, 2024

Changes proposed in this Pull Request:

It fixes the Hover Reveal style of the Cover block for the latest versions of WordPress.

Known issues:

#8386

Test instructions:

I tested upgrading to all major releases from 5.0 (5.1, 5.2, 5.3...). And I tried to simplify it getting some specific versions. But feel free to test bumping 1 in 1 if you have time. :)

  • Setup an environment with WP 5.0 (first version with Gutenberg).
  • Create a page with some cover blocks with some variations:
    • With an image in the background.
    • With a color.
    • Image and color.
    • Video and color.
  • Set the style "Hover Reveal" to the blocks.
  • Check that it works properly when you hover the elements.
  • Update your WP to 5.5.
  • Add more variations with gradient (using image, video and not using it), and with some opacity variations.
  • Check that it works properly when you hover the elements.
  • Update to WP 5.8 and re-create the blocks just to make sure the tests will still be reliable. Even not breaking, I noticed an error in the browser console after this upgrade.
  • Upgrade to WP 5.9, and see that the background will always be visible. Regardless if we are in this branch or trunk. I tried to fix it, but it was complex given all possible scenarios and I was also afraid to break for other versions, so I just created an issue for now since the more important would be to fix it for the latest versions.

Related issue(s):

#8377

The background element is child of the cover block element.
wp-block-cover__background is child of wp-block-cover.
The other styles are already affecting the editor selectors.
Gradients are impacted by the other selector since it's not a child of the wp-block-cover__background
@renatho renatho self-assigned this Nov 7, 2024
Copy link
Contributor

github-actions bot commented Nov 7, 2024

Preview changes

I've detected changes to the following themes in this PR: Dalston.
You can preview these changes by following the links below:

I will update this comment with the latest preview links as you push more changes to this PR.

Note

The preview sites are created using WordPress Playground. You can add content, edit settings, and test the themes as you would on a real site, but please note that changes are not saved between sessions.
Child themes are dependent on their parent themes. You will have to install the parent theme as well for the preview to work correctly.

@@ -144,23 +144,27 @@
text-decoration: underline;
}

.wp-block-cover.is-style-hover-reveal .wp-block-cover__inner-container {
.wp-block-cover.is-style-hover-reveal .wp-block-cover__inner-container,
.wp-block-cover.is-style-hover-reveal .wp-block-cover-text {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The addition of .wp-block-cover-text is to fix the reveal in old versions of Gutenberg where it didn't have the InnerBlocks in the Cover yet.

@@ -2,10 +2,10 @@
* Editor
*/

#editor .wp-block-cover-image.is-style-hover-reveal.has-background-dim.has-background-dim .wp-block-cover__gradient-background,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just remove the not needed duplicate selector .has-background-dim.has-background-dim.

opacity: 1;
}

/* Dim */

.wp-block-cover.is-style-hover-reveal.has-background-dim:before,
.wp-block-cover.is-style-hover-reveal.has-background-dim:not(.has-background-gradient):before,
.wp-block-cover.is-style-hover-reveal.has-background-dim.has-background-gradient .wp-block-cover__gradient-background {
.wp-block-cover.is-style-hover-reveal.has-background-dim.has-background-gradient .wp-block-cover__gradient-background,
.wp-block-cover.is-style-hover-reveal .wp-block-cover__background.has-background-dim {
Copy link
Contributor Author

@renatho renatho Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current Gutenberg version the has-background-dim class is applied to the element .wp-block-cover__background and not to the .wp-block-cover.

Copy link
Contributor

github-actions bot commented Nov 7, 2024

Theme-Check results

dalston: There are required changes on the theme ❌.

❎ REQUIRED

  • Could not find add_theme_support( 'automatic-feed-links' ). See: add_theme_support
  • bump-version-numbers.sh rebuild-child-themes.sh Shell script file found. This file must not be in the production version of the theme.
  • Screenshot is wrong size! Detected: 1800x1350. Maximum allowed size is 1200x900px.
  • Tested up to: is missing from your style.css header. Also, this should be numbers only, so 5.0 and not WP 5.0
  • Requires PHP: is missing from your style.css header.
  • No reference to add_theme_support( "title-tag" ) was found in the theme.
  • The theme appears to use dynamic_sidebars() but no register_sidebar() was found. See: register_sidebar
💡 RECOMMENDED (13)
  • No reference to register_block_pattern was found in the theme. Theme authors are encouraged to implement custom block patterns as a transition to block themes.
  • No reference to register_block_style was found in the theme. Theme authors are encouraged to implement new block styles as a transition to block themes.
  • Could not find the comment-reply script enqueued.
  • No reference to post-thumbnails was found in the theme. If the theme has a thumbnail like functionality, it should be implemented with add_theme_support( "post-thumbnails" ) in the functions.php file.
  • Screenshot size should be 1200x900, to account for HiDPI displays. Any 4:3 image size is acceptable, but 1200x900 is preferred.
  • .wp-caption css class is recommended in your theme css.
  • .wp-caption-text css class is recommended in your theme css.
  • No reference to add_theme_support( "custom-header", $args ) was found in the theme. It is recommended that the theme implement this functionality if using an image for the header.
  • No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
  • No reference to add_theme_support( "html5", $args ) was found in the theme. It is strongly recommended that the theme implement this functionality.
  • No reference to add_theme_support( "responsive-embeds" ) was found in the theme. It is recommended that the theme implement this functionality.
  • No reference to add_theme_support( "align-wide" ) was found in the theme. It is recommended that the theme implement this functionality.
  • No reference to add_theme_support( "wp-block-styles" ) was found in the theme. It is recommended that the theme implement this functionality.
⚠️ WARNING (15)
  • Found PHP short tags in file inc/wpcom.php. Line 2: <?
  • Found a translation function that is missing a text-domain in the file inc/wpcom-colors.php. Function __, with the arguments 'Background Color'. Line 138: ), __( 'Background Color' ) );
  • Found a translation function that is missing a text-domain in the file inc/wpcom-colors.php. Function __, with the arguments 'Link Color'. Line 270: ), __( 'Link Color' ) );
  • Found a translation function that is missing a text-domain in the file inc/wpcom-colors.php. Function __, with the arguments 'Text Color'. Line 403: ), __( 'Text Color' ) );
  • Found a translation function that is missing a text-domain in the file inc/wpcom-colors.php. Function __, with the arguments 'Secondary Color'. Line 423: ), __( 'Secondary Color' ) );
  • Found a translation function that is missing a text-domain in the file inc/wpcom-editor-colors.php. Function __, with the arguments 'Background Color'. Line 66: ), __( 'Background Color' ) );
  • Found a translation function that is missing a text-domain in the file inc/wpcom-editor-colors.php. Function __, with the arguments 'Link Color'. Line 100: ), __( 'Link Color' ) );
  • Found a translation function that is missing a text-domain in the file inc/wpcom-editor-colors.php. Function __, with the arguments 'Text Color'. Line 156: ), __( 'Text Color' ) );
  • Found a translation function that is missing a text-domain in the file inc/wpcom-editor-colors.php. Function __, with the arguments 'Secondary Color'. Line 173: ), __( 'Secondary Color' ) );
  • Found a translation function that is missing a text-domain in the file inc/wpcom-colors-css-variables.php. Function __, with the arguments 'Background Color'. Line 22: __( 'Background Color' )
  • Found a translation function that is missing a text-domain in the file inc/wpcom-colors-css-variables.php. Function __, with the arguments 'Foreground Color'. Line 38: __( 'Foreground Color' )
  • Found a translation function that is missing a text-domain in the file inc/wpcom-colors-css-variables.php. Function __, with the arguments 'Primary Color'. Line 54: __( 'Primary Color' )
  • Found a translation function that is missing a text-domain in the file inc/wpcom-colors-css-variables.php. Function __, with the arguments 'Secondary Color'. Line 70: __( 'Secondary Color' )
  • Found a translation function that is missing a text-domain in the file inc/wpcom-colors-css-variables.php. Function __, with the arguments 'Tertiary Color'. Line 86: __( 'Tertiary Color' )
  • More than one text-domain is being used in this theme. This means the theme will not be compatible with WordPress.org language packs. The domains found are dalston, varia.

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.

1 participant