wpengine / wp-graphql-content-blocks

Plugin that extends WPGraphQL to support querying (Gutenberg) Blocks as data
https://faustjs.org/docs/gutenberg/wp-graphql-content-blocks
GNU General Public License v2.0
107 stars 14 forks source link

CoreImage give wrong caption when lightbox is enable #185

Open MKlblangenois opened 8 months ago

MKlblangenois commented 8 months ago

When I add caption on Image, if "lightbox" checkbox is checked, graphql show me the caption three times.

SCR-20240206-ixaf

Step to reproduce:

  1. Add a block "image" on your gutenberg and add a caption
  2. Duplicate it and enable "lightbox on click" checkbox
  3. Make the following graphql request :
    {
    page(id: "your-page-slug", idType: URI) {
    editorBlocks(flat: false) {
      ... on CoreImage {
        attributes {
          caption
          lightbox
        }
      }
    }
    }
    }

Version:

theodesp commented 8 months ago

Hey @MKlblangenois I was able to reproduce.

Unfortunately the problem lies with the way the Gutenberg uses the selectors to specify specificity.

In our case the attribute definition for caption is:

"caption": {
    "type": "string",
    "source": "html",
    "selector": "figcaption",
    "__experimentalRole": "content"
  },

However when you enable the lightbox, the selector for this will match multiple figcaption elements. Here is the HTML of the Image block when using this selector:

<figure>
  <img data-wp-effect--setstylesonresize=\"effects.core.image.setStylesOnResize\" data-wp-effect=\"effects.core.image.setButtonStyles\" data-wp-init=\"effects.core.image.initOriginImage\" data-wp-on--click=\"actions.core.image.showLightbox\" data-wp-on--load=\"actions.core.image.handleLoad\" src=\"http://mysite.local/wp-content/uploads/2023/09/pexels-evg-kowalievska-1170986-683x1024.jpg\" alt=\"A Caption\" class=\"wp-image-2043\" title=\"caption\">
  <button class=\"lightbox-trigger\" type=\"button\" aria-haspopup=\"dialog\" aria-label=\"Enlarge image: A Caption\" data-wp-on--click=\"actions.core.image.showLightbox\" data-wp-style--right=\"context.core.image.imageButtonRight\" data-wp-style--top=\"context.core.image.imageButtonTop\">\n\t\t\t <svg xmlns=\"http://www.w3.org/2000/svg\" width=\"12\" height=\"12\" fill=\"none\" viewbox=\"0 0 12 12\">\n\t\t\t\t <path fill=\"#fff\" d=\"M2 0a2 2 0 0 0-2 2v2h1.5V2a.5.5 0 0 1 .5-.5h2V0H2Zm2 10.5H2a.5.5 0 0 1-.5-.5V8H0v2a2 2 0 0 0 2 2h2v-1.5ZM8 12v-1.5h2a.5.5 0 0 0 .5-.5V8H12v2a2 2 0 0 1-2 2H8Zm2-12a2 2 0 0 1 2 2v2h-1.5V2a.5.5 0 0 0-.5-.5H8V0h2Z\"></path>\n\t\t\t </svg>\n\t\t </button>** <figcaption class=\"wp-element-caption\">caption</figcaption>** <div data-wp-body=\"\" class=\"wp-lightbox-overlay zoom\" data-wp-bind--role=\"selectors.core.image.roleAttribute\" data-wp-bind--aria-label=\"selectors.core.image.dialogLabel\" data-wp-class--initialized=\"context.core.image.initialized\" data-wp-class--active=\"context.core.image.lightboxEnabled\" data-wp-class--hideanimationenabled=\"context.core.image.hideAnimationEnabled\" data-wp-bind--aria-modal=\"selectors.core.image.ariaModal\" data-wp-effect=\"effects.core.image.initLightbox\" data-wp-on--keydown=\"actions.core.image.handleKeydown\" data-wp-on--touchstart=\"actions.core.image.handleTouchStart\" data-wp-on--touchmove=\"actions.core.image.handleTouchMove\" data-wp-on--touchend=\"actions.core.image.handleTouchEnd\" data-wp-on--click=\"actions.core.image.hideLightbox\" tabindex=\"-1\">\n <button type=\"button\" aria-label=\"Close\" style=\"fill: var(--wp--preset--color--contrast)\" class=\"close-button\" data-wp-on--click=\"actions.core.image.hideLightbox\">\n <svg xmlns=\"http://www.w3.org/2000/svg\" viewbox=\"0 0 24 24\" width=\"20\" height=\"20\" aria-hidden=\"true\" focusable=\"false\">
        <path d=\"M13 11.8l6.1-6.3-1-1-6.1 6.2-6.1-6.2-1 1 6.1 6.3-6.5 6.7 1 1 6.5-6.6 6.5 6.6 1-1z\"></path>
      </svg>\n </button>\n <div class=\"lightbox-image-container\">\n <figure class=\"wp-block-image size-large responsive-image\">
        <img data-wp-bind--src=\"context.core.image.imageCurrentSrc\" data-wp-style--object-fit=\"selectors.core.image.lightboxObjectFit\" src=\"\" alt=\"A Caption\" class=\"wp-image-2043\" title=\"caption\">
        <figcaption class=\"wp-element-caption\">caption</figcaption>
      </figure>\n </div>\n <div class=\"lightbox-image-container\">\n <figure class=\"wp-block-image size-large enlarged-image\">
        <img data-wp-bind--src=\"selectors.core.image.enlargedImgSrc\" data-wp-style--object-fit=\"selectors.core.image.lightboxObjectFit\" src=\"\" alt=\"A Caption\" class=\"wp-image-2043\" title=\"caption\">
        <figcaption class=\"wp-element-caption\">caption</figcaption>
      </figure>\n </div>\n <div class=\"scrim\" style=\"background-color: var(--wp--preset--color--base)\" aria-hidden=\"true\"></div>\n </div>
  <img data-wp-bind--src=\"context.core.image.imageCurrentSrc\" data-wp-style--object-fit=\"selectors.core.image.lightboxObjectFit\" src=\"\" alt=\"A Caption\" class=\"wp-image-2043\" title=\"caption\">
  <figcaption class=\"wp-element-caption\">caption</figcaption>
  <img data-wp-bind--src=\"selectors.core.image.enlargedImgSrc\" data-wp-style--object-fit=\"selectors.core.image.lightboxObjectFit\" src=\"\" alt=\"A Caption\" class=\"wp-image-2043\" title=\"caption\">** <figcaption class=\"wp-element-caption\">caption</figcaption>
</figure>**

If you observe carefully you can see that there are three instances of figcaption that can be matched with this selector so there are three text contents that will be added together.

However in that case I propose we fix the selector so to find only the first child element in the list instead of all figcaption elements. To do that we can to allow using Xpath as selectors. In our case the correct selector would be:

"caption": {
    "type": "string",
    "source": "html",
    "selector": "(//figcaption)[1]",
    "__experimentalRole": "content"
  },

So what I will do is add a feature request to allow Xpath selectors to be used so that we can extract more accurately the contents of this field.

Then we can update the block image attribute to use this new selector type.

justlevine commented 8 months ago

edit: relocated to #186 .

To do that we can to allow using Xpath as selectors.

@theodesp I havn't been following this issue too closely, but please keep in mind the general changes to the Block API as you're making these decisions.

Between the Interactivity API and Block Bindings API coming up, my gut tells me that we want to be using the HTML parser (assuming there isn't something block-specific to handle Lightbox Images, and to stay away from using Xpath directly.