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

Added block and content resolver for CoreFootnotes #335

Closed
wants to merge 2 commits into from

Conversation

colinmurphy
Copy link
Contributor

Summary

This PR adds a content block for CoreFootnotes and also adds a post content resolver to load the post meta data for footnotes and also parse the HTML.

Example

Query

fragment CoreFootnotesBlockFragment on CoreFootnotes {
  innerBlocks {
    renderedHtml
  }
}

query Post($id: ID!) {
  post(id: $id, idType: DATABASE_ID) {
    databaseId
    editorBlocks {
      ...CoreFootnotesBlockFragment
    }
  }
}

Example Response

{
  "data": {
    "post": {
      "databaseId": 16,
      "editorBlocks": [
        {},
        {
          "innerBlocks": [
            {
              "renderedHtml": "<ol class=\"footnotes\"><li id=\"d4051e5e-1547-49ff-ab6d-bec1caa6aabc\"><a href=\"https://wpengine.com/about-us/\">https://wpengine.com/about-us/</a></li><li id=\"2e79de23-68a8-42eb-87ab-1f2467a21752\"><a href=\"https://wpengine.com/support/\">https://wpengine.com/support/</a></li></ol>"
            }
          ]
        },
        {}
      ]
    }
  },
  "extensions": {
    "debug": [],
    "queryAnalyzer": {
      "keys": "262c7066ca9adad309e85fa6e74f44cbba09adbaf24eba42ac58b418899a687f graphql:Query operation:Post cG9zdDoxNg==",
      "keysLength": 106,
      "keysCount": 4,
      "skippedKeys": "",
      "skippedKeysSize": 0,
      "skippedKeysCount": 0,
      "skippedTypes": []
    }
  }
}

Added content block resolver for getting post meta for footnotes.
@colinmurphy colinmurphy requested a review from a team as a code owner January 20, 2025 18:41
Copy link

changeset-bot bot commented Jan 20, 2025

🦋 Changeset detected

Latest commit: d1ce72c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@wpengine/wp-graphql-content-blocks Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -152,6 +152,7 @@ private static function handle_do_block( array $block ): ?array {
$block = self::populate_post_content_inner_blocks( $block );
$block = self::populate_reusable_blocks( $block );
$block = self::populate_pattern_inner_blocks( $block );
$block = self::populate_core_footnotes_inner_blocks( $block );
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

@theodesp theodesp Jan 21, 2025

Choose a reason for hiding this comment

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

I was actually able to render the footnotes HTML without checking out this branch. This is on the main branch:

{
  posts {
    nodes {
      editorBlocks {
        ...on CoreFootnotes {
          type
          renderedHtml
        }
      }
    }
  }
}
{
  "data": {
    "posts": {
      "nodes": [
        {
          "editorBlocks": [
            {
              "type": "CoreFootnotes",
              "renderedHtml": "<ol class=\"wp-block-footnotes\"><li id=\"183aa597-72fa-49da-8ea2-73aff3387d88\">Another footnote <a href=\"#183aa597-72fa-49da-8ea2-73aff3387d88-link\" aria-label=\"Jump to footnote reference 1\">↩︎</a></li><li id=\"d6700ca9-32d4-4f18-ad7d-0f6b00d99137\">footnote 1 <a href=\"#d6700ca9-32d4-4f18-ad7d-0f6b00d99137-link\" aria-label=\"Jump to footnote reference 2\">↩︎</a></li></ol>"
            },

          ]
        },
       }
}

Do we really need to expose the individual footnote items from this list? I'm not sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@theodesp @justlevine I think if we can get the data (as per @theodesp GQL query) then is there any need for this? I don't think we should add any new blocks/logic if we can already get the data? The other thing is footnotes are directly correlated with content so not sure if adding a new block for footnotes is needed on reflection.

Copy link
Contributor

Choose a reason for hiding this comment

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

agree there's no need, and personally don't see the value, especially versus the cost of maintaining this (and any other manual deviation that we cant auto-infer from WP core in our schema generation)

Unit tests are always nice to alert us if renderedHtml or something else changes in the future, but I don't see that in this diff anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @justlevine

I don't see any point either if we can just query the data. I will close this PR out. Thanks for the really good feedback aswell.

Co-authored-by: Dovid Levine <david@axepress.dev>
@colinmurphy colinmurphy marked this pull request as draft January 21, 2025 16:09
@colinmurphy
Copy link
Contributor Author

Closing as not needed.

@moonmeister moonmeister deleted the feature-core-footnotes branch January 21, 2025 16:50
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.

None yet

3 participants