WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.4k stars 4.16k forks source link

Image lightbox: configure lightbox settings in `theme.json` at the top-level #54858

Open oandregal opened 1 year ago

oandregal commented 1 year ago

Description

The lightbox feature can be controlled via theme.json with two settings: allowEditing to hide/show the UI panel) and enabled (to control whether it is enabled or not).

While configuring the value of these settings at the block-level works as expected, doing the same at the top-level does not work.

Step-by-step reproduction instructions

BLOCK LEVEL. Test the UI can be disabled and the feature enabled (works as expected):

{
  "settings": {
    "blocks": {
      "core/image": {
        "lightbox": {
          "allowEditing": false,
          "enabled": true
        }
      }
    }
  }
}

The expected behaviour is that:

TOP-LEVEL. Test the UI can be disabled at the feature enabled (does not work):

{
  "settings": {
    "lightbox": {
      "allowEditing": false,
      "enabled": true
    }
  }
}

The expected behaviour is that:

Screenshots, screen recording, code snippet

This is the UI for the lightbox feature, when is visible:

Post editor (block inspector of the image block) Site editor (global styles sidebar > blocks > image)
Captura de ecrã 2023-09-27, às 12 30 07 Captura de ecrã 2023-09-27, às 12 31 58

Environment info

WordPress 6.4 beta1, Gutenberg trunk (16.7.0-rc.2), TwentyTwentyThree theme. I could replicate the issue with and without Gutenberg enabled.

oandregal commented 1 year ago

cc @artemiomorales @michalczaplinski

artemiomorales commented 1 year ago

Duplicate of https://github.com/WordPress/gutenberg/issues/54544

michalczaplinski commented 1 year ago

TOP-LEVEL. Test the UI can be disabled at the feature enabled (does not work):

  • Edit the theme.json of the active theme such as:
{
  "settings": {
    "lightbox": {
      "allowEditing": false,
      "enabled": true
    }
  }
}

The expected behaviour is that:

  • the lightbox feature is not visible in the post editor or in the site editor
  • the lightbox feature is enabled in the front (the image expands upon clicking)

The issue here is that in the core theme.json the lightbox is defined on the block-level:

https://github.com/WordPress/gutenberg/blob/5b286b720c14ae2f9c5832954a3c0f1d9a5fa13a/lib/theme.json#L276-L279

So, setting in on the top-level in the settings does not override it. The obvious solution would be to define it on the top-level in the Core theme.json but there was another issue with that approach. I forget what it was exactly - do you remember, @artemiomorales ?

artemiomorales commented 1 year ago

The obvious solution would be to define it on the top-level in the Core theme.json but there was another issue with that approach. I forget what it was exactly - do you remember, @artemiomorales ?

@michalczaplinski The issue was that, when using the shorthand syntax, i.e. settings.lightbox = true the existing logic was unable to merge that boolean with a block-level lightbox setting, an object, if a user were to set it.

Since we're not planning on supporting the shorthand syntax for the time being, that is no longer an issue. The issue with the false values not being saved was also presenting inconsistencies, though that's now been merged.

There may have been other issues with it as well, but those are the primary ones that comes to mind.

michalczaplinski commented 1 year ago

Let's close the other issue (#54544) in favor of this one because it has more details and discussion now 🙂

when using the shorthand syntax Riiight, it was the shorthand syntax issue.

I've just tested the approach of adding moving the lightbox definition to the top level and while it does solve the problem described in the issue, it causes the inverse problem: Now it's not possible to define properties on the block-level in the theme.json. 😃

It's something that we've seen previously with Artemio, I'm not sure what we're doing wrong, but needs a deeper look.

michalczaplinski commented 11 months ago

@artemiomorales Do you still expect we ship this for 6.4?

artemiomorales commented 11 months ago

@michalczaplinski It was lowest on the list of priorities as far as the bugs go, and it looks like I won't have time to work on it in the next couple of days. So it looks like probably not unless you basically already have a fix ready to go. From what I understand, shipping this in the next minor release is fine, so I'd focus efforts on other more pressing bugs if you're able.

michalczaplinski commented 11 months ago

All right, that makes sense. I didn't have the time to look into it in depth, I'm sorry.

We can ship it in the next minor release 👍

oandregal commented 6 months ago

I see this is still not possible to do in 6.5. Any chance this can be fixed for 6.6? @artemiomorales @michalczaplinski

artemiomorales commented 6 months ago

@oandregal Thanks for the ping. I'll add it to the list of tasks for 6.6 and prioritize relative to other projects for the coming weeks.

nderambure commented 2 months ago

Hi @artemiomorales, do you have any news about this ?

6.6 is out and set "lightbox" properties all to false in settings section or in blocks > core/img does not hide the functionnality from link button in an image block.

Thx !

michalczaplinski commented 2 months ago

👋 @nderambure no news yet, but @artemiomorales and I will take another look at this tomorrow! 🙂

michalczaplinski commented 2 months ago

Hi @artemiomorales, do you have any news about this ?

6.6 is out and set "lightbox" properties all to false in settings section or in blocks > core/img does not hide the functionnality from link button in an image block.

Thx !

👋 @nderambure

It sounds like your problem is unrelated to the issue described here originally.

Could you open a new issue and provide more details that would allow us to replicate your problem? It would be especially helpful to see your theme.json file and the HTML of the blocks where the lightbox functionality still appears despite being disabled.