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 md5 class messed up with new block key #52557

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

luisherranz
Copy link
Member

What?

Fix a mismatch between the class added to some blocks and the styles added to the page.

Fixes #52195.

Why?

Because the styles engine is using an md5 of the block array, and adding keys to the array (like $block['is_inner_block]) changes the md5.

How?

Instead of adding a new key to $block, I used the same md5 technique to keep an array of root blocks.

Testing Instructions

Check that the md5-generated class works again:

  • Add this markup to a post:
    <!-- wp:query {"queryId":1,"query":{"offset":0,"postType":"post","order":"desc","orderBy":"date","author":"","search":"","sticky":"","perPage":10}} -->
    <div class="wp-block-query">
      <!-- wp:post-template {"layout":{"type":"default"}} -->
      <!-- wp:post-title {"isLink":true,"style":{"elements":{"link":{"color":{"text":"var:preset|color|vivid-red"}}}}} /-->
      <!-- wp:post-excerpt /-->
      <!-- /wp:post-template -->
    </div>
    <!-- /wp:query -->
  • Save the post
  • Open the post in the frontend
  • Check that color of the post title is red

Check that the Interactivity API server-side rendering still works fine:

  • Open a post
  • Add an HTML block
  • Paste this code
    <div data-wp-context='{ "hide": false, "color": "red" }'>
      <div
        class="another"
        style="text-decoration: underline"
        data-wp-bind--hidden="context.hide"
        data-wp-class--show="!context.hide"
        data-wp-style--color="context.color"
        data-wp-text="context.color"
      >
        default value
      </div>
    </div>
  • Take a look at the frontend, there should be a red underlined text saying "red" with the class "show"
  • Change the "color" to "blue".
  • Now there should be a blue underlined text saying "blue"
  • Change "hide" to true.
  • The text now should have a hidden attribute and the "show" class should not be there anymore

@luisherranz luisherranz added [Type] Bug An existing feature does not function as intended [Package] Style Engine /packages/style-engine Backport to Gutenberg RC Pull request that needs to be backported to a Gutenberg release candidate (RC) [Feature] Interactivity API API to add frontend interactivity to blocks. labels Jul 12, 2023
@luisherranz luisherranz self-assigned this Jul 12, 2023
@github-actions
Copy link

Flaky tests detected in 083422e.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5529728371
📝 Reported issues:

@michalczaplinski
Copy link
Contributor

Nice! Tested and works as expected 👍

@michalczaplinski michalczaplinski merged commit b05c23b into trunk Jul 12, 2023
49 checks passed
@michalczaplinski michalczaplinski deleted the fix/md5-class-messed-up-with-new-block-key branch July 12, 2023 10:58
@github-actions github-actions bot added this to the Gutenberg 16.3 milestone Jul 12, 2023
Copy link
Contributor

@DAreRodz DAreRodz left a comment

Choose a reason for hiding this comment

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

I've just tested it in http://gutenberg.run, and it works as mentioned. 👍

The code looks good to me, although I'd prefer someone else to also take a look. 🙂 EDIT: Michal already reviewed, approved, and merged the PR. 😄

PS: I guess it is explained somewhere in the file what "root blocks" are, as we're already using that term in other functions. They are the root block of a block tree containing directives, right?

@luisherranz
Copy link
Member Author

I guess it is explained somewhere in the file what "root blocks" are, as we're already using that term in other functions. They are the root block of a block tree containing directives, right?

It's a temporary measure to process the directives, but it's not even the correct one because we are processing the blocks multiple times and also processing them as part of a parent block where the search for the data-wp- optimization doesn't make much sense. We need to study the render pipeline and find the best place to process the directives, so they are processed only once and only their HTML is processed (and not their children's).

@ockham
Copy link
Contributor

ockham commented Jul 12, 2023

Cherry-picked to release/16.2 in cbec878.

@ockham ockham removed the Backport to Gutenberg RC Pull request that needs to be backported to a Gutenberg release candidate (RC) label Jul 12, 2023
westonruter added a commit that referenced this pull request Jul 12, 2023
…dd/defer-script-loading-strategy

* 'trunk' of https://github.com/WordPress/gutenberg:
  Update Changelog for 16.2.0
  Adding support for defined IDs in `TextControl` component (#52028)
  Bump plugin version to 16.2.0
  Revert "Bump plugin version to 16.2.0"
  Bump plugin version to 16.2.0
  Add maxLength to LinkControl search items (#52523)
  [RNMobile] Update Editor block inserter button styles and default text input placeholder/selection styles (#52269)
  Site Editor: Reset device preview type when exiting the editing mode (#52566)
  Trim footnote anchors from excerpts (#52518)
  Add back old Navigation and File blocks JavaScript implementation when Gutenberg is not installed (#52553)
  Block Editor: Ensure synced patterns are accounted for in 'getAllowedBlocks' (#52546)
  Fix md5 class messed up with new block key (#52557)
  Fix entity cache misses for single posts due to string as recordKey (#52338)
  Make "My patterns" permanently visible (#52531)
  Hide site hub when resizing frame upwards to avoid overlap (#52180)
  Fix "Manage all patterns" link appearance (#52532)
  Update navigation menu title size & weight in detail panels (#52477)
  Site Editor Patterns: Ensure sidebar does not shrink when long pattern titles are used (#52547)
  Site Editor: Restore quick inserter 'Browse all' button (#52529)
  Patterns: update the title of Pattern block in the block inspector card  (#52010)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Interactivity API API to add frontend interactivity to blocks. [Package] Style Engine /packages/style-engine [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Post Title: link color not applied in the view
5 participants