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

Jetpack Tiled Gallery: Remove/Hide Image Settings and Caption #4190

Closed
SiobhyB opened this issue Nov 2, 2021 · 6 comments · Fixed by WordPress/gutenberg#36618 or #4256
Closed

Jetpack Tiled Gallery: Remove/Hide Image Settings and Caption #4190

SiobhyB opened this issue Nov 2, 2021 · 6 comments · Fixed by WordPress/gutenberg#36618 or #4256
Assignees
Labels
InnerBlocks [Pri] High Tiled Gallery block Issues with this label will automatically appear on the project's board [Type] Bug Something isn't working

Comments

@SiobhyB
Copy link
Contributor

SiobhyB commented Nov 2, 2021

Describe the bug

Images are pulled into the Jetpack Tiled Gallery block via the inner blocks API. As a result of this implementation, all of the image block's settings are available by default, including its settings tab and editable caption field.

For the purpose of the Tiled Gallery block, it shouldn't be possible for users to edit an individual image's settings or caption. The reason for this is that this would add styles/attributes to the block's HTML and the web wouldn't know how to interpret this output.

To Reproduce

Steps to reproduce the behavior:

  1. Add the Tiled Gallery block to the demo app and upload a few images to it.
  2. Tap on any individual image and observe how it's possible to edit it via the settings tab or the editable caption field.

Expected behavior

It shouldn't be possible to access settings for individual images within the Tiled Gallery. Instead, mobile should match up with the web's behaviour, where individual settings and caption fields aren't accessible.

Smartphone (please complete the following information):

  • Device: Pixel 5
  • OS: Android 12
  • Version: Demo app

Additional context

A solution to this is likely to require block contexts. However, contexts for a third-party block have never been added before. If we do use contexts, we should ensure that they wouldn't only be useful for the Tiled Gallery block. They'd need to have a clear use for other third-parties who may wish to create a block using images as inner blocks.

In #4099, a proposal for "context constants" has been made. The idea behind these "context constants" is to better indicate when contexts aren't being serialised, and are primarily used for the purpose of styling or changing specific aspects of an inner block. If approved, it might be more straightforward to add contexts the type of contexts we're looking to add to address this issue.

A short-term workaround, in case the idea of contexts isn't approved, could be using CSS to hide these settings instead. This will need to be explored further.

@SiobhyB SiobhyB added [Type] Bug Something isn't working InnerBlocks Jetpack Bug or feature related to Jetpack Tiled Gallery block Issues with this label will automatically appear on the project's board and removed Jetpack Bug or feature related to Jetpack labels Nov 2, 2021
@SiobhyB SiobhyB self-assigned this Nov 2, 2021
@guarani
Copy link
Contributor

guarani commented Nov 5, 2021

Hi @SiobhyB, do you agree we could close #3907 now that the aspect ratio issue (#4010) is resolved and this issue tracks hiding inner image block settings and captions?

Also, while this issue suggests using block contexts to solve the problem of hiding this functionality, we've recently discussed using CSS to achieve this in the short-term. I wonder if this issue should be updated to reflected the CSS approach and a new one created for revisiting this in the future.

@SiobhyB
Copy link
Contributor Author

SiobhyB commented Nov 5, 2021

@guarani, I agree it makes sense to close #3907 now, I'll go ahead to do that. :)

Also, while this issue suggests using block contexts to solve the problem of hiding this functionality, we've recently discussed using CSS to achieve this in the short-term.

Great point! I'll update the issue to reflect our most recent conversations.

@SiobhyB
Copy link
Contributor Author

SiobhyB commented Nov 5, 2021

I wonder if this issue should be updated to reflected the CSS approach and a new one created for revisiting this in the future.

Sounds good, I'll see how far I'm able to get with the CSS approach and create a second ticket for revisiting if I'm able to get something merged out of it 👍

@guarani
Copy link
Contributor

guarani commented Nov 24, 2021

👋 @SiobhyB, when testing the Gutenberg Mobile feature branch, add/tiled-gallery-block, the captions are still visible on the Tiled Gallery inner Image blocks. It looks like the PRs that closed this issue target the repo's main branches instead of our feature branches.

It looks like the best way to fix this is to simply update our feature branches with the latest from their repo's main branches, would you agree? If so, would you like to do that when you have a chance? It shouldn't need a PR I don't think.

@SiobhyB
Copy link
Contributor Author

SiobhyB commented Nov 25, 2021

Thanks for the ping @guarani! You're right that I intentionally targeted the main branches of the repos with the others PRs, but forgot to update our feature branches. 🤦‍♀️ Updated the branches now!

@guarani
Copy link
Contributor

guarani commented Nov 25, 2021

I intentionally targeted the main branches of the repos with the others PRs

Gotcha, makes sense. Thanks for updating the feature branches!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
InnerBlocks [Pri] High Tiled Gallery block Issues with this label will automatically appear on the project's board [Type] Bug Something isn't working
Projects
None yet
3 participants