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

Move "Expand on click" to the image link control #55020

Closed
richtabor opened this issue Oct 3, 2023 · 10 comments
Closed

Move "Expand on click" to the image link control #55020

richtabor opened this issue Oct 3, 2023 · 10 comments
Labels
[Block] Image Affects the Image Block Needs Dev Ready for, and needs developer efforts [Type] Enhancement A suggestion for improvement.

Comments

@richtabor
Copy link
Member

As a follow-up to #54916 (comment) (and related to #50893) let's move the lightbox "Expand on click" control from the inspector to the existing link action control.

Why? You can only have one of these options selected at a time. Connecting them into one singular (and familiar) experience will help reduce confusion when attempting to link an image with the lightbox effect applied (or vise-versa).

Visuals

  1. The LinkControl implementation for the image block, as detailed in Image and Media & Text blocks: Use LinkControl #50893, with the new control for "Expand on click".
  2. The selected state indicating that the click action is set to "Expand on click". Removing should not close the link modal, but bring it back to the initial state above.
3 5
@richtabor
Copy link
Member Author

richtabor commented Oct 3, 2023

@jasmussen What do you think about using the checkmark to indicate selection, when it's not a token? Fell into that with the exploration on consolidating gallery image linking in #55022.

CleanShot 2023-10-03 at 16 27 59

@paaljoachim
Copy link
Contributor

This is very interesting!
Thank you for adding this issue, Rich!
Is the checkmark used in any other UX in Gutenberg?
It seems a few places the icon becomes black when in use. Be it alignment or in the More drop down.
In this case color the expand icon black might not be clear enough. So having a checkmark or something like it seems interesting.

@getdave
Copy link
Contributor

getdave commented Oct 4, 2023

I appreciate the effort that has gone into this both here and in discussions in #54916.

I would like to restate my previous concerns that using these "suggestions" in the Link UI is going against the grain of how the UX of the control is designed. Why? Because these are search suggestions and not configuration options.

I have a counter proposal which I would appreciate folks considering.

Why not simply default the Link UI to have the current URL of the image pre-selected in the Link UI by default. Then these options:

  • Link to Image File
  • Link to Attachment
  • Expand on click

...could be radio buttons in the Advanced section of the Link UI which is specifically designed to handle this type of configuration option.

If necessary we can change the behaviour to allow us to force the Advanced panel to be open or something to ensure these are exposed by default and are not hidden.

I trust it's at least worth exploring this as an alternative before we press on with a route which will require considerable changes to the underlying components to get right. This could end up expending a lot of effort so I want to ensure we are all confident before we proceed here 🙏

Thanks in advance.

@jasmussen
Copy link
Contributor

...could be radio buttons in the Advanced section of the Link UI which is specifically designed to handle this type of configuration option.

This is actually what we want to avoid.

The thing is, Lightbox is something that you enable once globally, and when you do, conceptually two things happen:

  1. attachment pages get disabled, if not de-facto then at least conceptually lightbox is a replacement for it
  2. all images are now effectively linked by default

2 is the important piece here, since effectively the image will have a linked behavior in that it links to a larger version of the image, even if the lightbox behavior makes it an inline experience. This is especially important for the user flow: people will experience how clicking an image does something, and therefore intuit that in order to change this behavior they must look in the linking interface.

This is what provides us an opportunitey to surface the lightbox option here, which comes with a few benefits beyond the discoverability:

  • It implies, by having to untoggle the lightbox first, that lightbox is mutually exclusive from linking to a destination.
  • It surfaces what can otherwise be a very hidden behavior, and puts it front and center.

I also don't share your concern that these suggestions go against the grain of what we're doing. Page links, attachment links, those should increasingly become tokens rather than always just be fixed URLs. Lighbox can be thought of as just another token.

I think there's room for iteration, for a step 1 that we can do today. This is also because the amount of work to change this is something to consider.

@getdave
Copy link
Contributor

getdave commented Oct 4, 2023

Thanks for the detailed explanation.

Page links, attachment links, those should increasingly become tokens rather than always just be fixed URLs

I agree with this. I'm working towards exactly this experience in #46891.

That said, I'm not yet fully convinced that selecting a single type of "token" outlined above and the proposed UI of selecting between multiple options is the same action. I'm willing to suspend my skepticism and iterate however 😄

My main concern would be if we went with a radio button like approach as shown in this mockup. That does go against the grain of the UX as nowhere else does this control behave in that manner and thus it's unexpected and requires users to learn a new UX pattern.

If we can make these options selectable but avoid making them <radio> (as per original description mockup) then it feels like we're in a good position. Saying "Your image links to a lightbox" seems reasonable.

Thanks for taking the time to go through this 🙇‍♂️

@richtabor
Copy link
Member Author

richtabor commented Oct 4, 2023

I trust it's at least worth exploring this as an alternative before we press on with a route which will require considerable changes to the underlying components to get right. This could end up expending a lot of effort so I want to ensure we are all confident before we proceed here

I see these three options following suite of the additional controls at the foot of the LinkControl UI, where we also have "Add new page", or "Add heading link" #53147, or "Add block" #50888. And link to image file and attachment page are already planned to move as such in #50893.

Perhaps the image block can pass its unique controls through to the LinkControl footer.

My main concern would be if we went with #55020 (comment). That does go against the grain of the UX as nowhere else does this control behave in that manner and thus it's unexpected and requires users to learn a new UX pattern.

Yea, that's just one exploration that makes more UX sense, but breaks the existing LinkControl model a bit. We could just stick with the "token" style, which would relay the currently selected option in the link preview, which would need to be "unlinked" to revert the LinkControl state back to initial.

CleanShot 2023-10-04 at 07 16 24

@hanneslsm
Copy link

Another thought in a different direction: What if the image that expands on click was a block variation of the image block?
This block variation simply does not have a link UI option.
If the Lightbox is globally deactivated, the "lightbox image block" acts as a regular image block.

@richtabor
Copy link
Member Author

What if the image that expands on click was a block variation of the image block?

Variations are usually visually distinct. I think it'd be confusing not having a distinction when all the others would.

@hanneslsm
Copy link

Variations are usually visually distinct. I think it'd be confusing not having a distinction when all the others would.

True, didn't think about this.

@jasmussen jasmussen added the Needs Dev Ready for, and needs developer efforts label Nov 21, 2023
@Mamaduka
Copy link
Member

I believe this was resolved in #57608 by @artemiomorales!

I'm going to close the issue.

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 Needs Dev Ready for, and needs developer efforts [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

6 participants