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

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

Open
oandregal opened this issue Sep 27, 2023 · 10 comments
Open
Assignees
Labels
[Block] Image Affects the Image Block [Type] Bug An existing feature does not function as intended

Comments

@oandregal
Copy link
Member

oandregal commented Sep 27, 2023

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

  • Open the post editor, add one image, and publish the post.

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

  • Edit the theme.json of the active theme such as:
{
  "settings": {
    "blocks": {
      "core/image": {
        "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)

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)

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 oandregal added [Type] Bug An existing feature does not function as intended [Block] Image Affects the Image Block [Feature] Interactivity API API to add frontend interactivity to blocks. labels Sep 27, 2023
@oandregal
Copy link
Member Author

cc @artemiomorales @michalczaplinski

@oandregal oandregal changed the title Configuring settings.lightbox.allowEditing to false in theme.json doesn't hide the UI setting Configure lightbox settings in theme.json at the top-level Sep 27, 2023
@oandregal oandregal changed the title Configure lightbox settings in theme.json at the top-level Image lightbox: configure lightbox settings in theme.json at the top-level Sep 27, 2023
@artemiomorales
Copy link
Contributor

Duplicate of #54544

@artemiomorales artemiomorales marked this as a duplicate of #54544 Sep 27, 2023
@michalczaplinski
Copy link
Contributor

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:

gutenberg/lib/theme.json

Lines 276 to 279 in 5b286b7

"core/image": {
"lightbox": {
"allowEditing": true
}

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

artemiomorales commented Sep 27, 2023

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.

@bph bph added Blessed for major release Can be iterated during a WordPress beta period and removed Blessed for major release Can be iterated during a WordPress beta period labels Sep 28, 2023
@michalczaplinski
Copy link
Contributor

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

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

@artemiomorales
Copy link
Contributor

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

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
Copy link
Member Author

oandregal commented Mar 25, 2024

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

@artemiomorales
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block [Type] Bug An existing feature does not function as intended
Projects
Status: Dev Backlog
Development

No branches or pull requests

5 participants