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

CoreImage give wrong caption when lightbox is enable #185

Open
MKlblangenois opened this issue Feb 6, 2024 · 2 comments
Open

CoreImage give wrong caption when lightbox is enable #185

MKlblangenois opened this issue Feb 6, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@MKlblangenois
Copy link

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:

  • wp-graphql-content-blocks: 2.0.0
  • Faust.js: 1.2.1
  • WPGraphQL: 1.20.0
@theodesp theodesp added enhancement New feature or request good first issue Good for newcomers labels Feb 6, 2024
@theodesp
Copy link
Member

theodesp commented Feb 6, 2024

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
Copy link
Contributor

justlevine commented Feb 6, 2024

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.

@theodesp theodesp removed enhancement New feature or request good first issue Good for newcomers labels Feb 6, 2024
@mindctrl mindctrl added the bug Something isn't working label Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants