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

New Lightbox with display bug in Classic Themes #54682

Closed
burnuser opened this issue Sep 21, 2023 · 11 comments · Fixed by #54837
Closed

New Lightbox with display bug in Classic Themes #54682

burnuser opened this issue Sep 21, 2023 · 11 comments · Fixed by #54837
Assignees
Labels
[Block] Image Affects the Image Block [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@burnuser
Copy link

Description

Image Lightbox (expand on click) in Gutenberg 16.7.0 RC1 works as expected with Theme Twenty Twenty-Two

After changing to classic Theme Twenty Sixteen (or GeneratePress) an error occurs:
Warning: Array to string conversion in ...\wp-includes\formatting.php on line 1104

Checked:
grafik

Visual bug:

  1. Image looks regular
  2. on mouseover: image disappears and is replaced with a monochrome coloured area of same size
  3. click on image opens lightbox as expected
  4. click again closes lightbox as expected
  5. image was again replaced with coloured area (even without mouseover!)
  6. clicking elsewhere make image reappear like regular

Step-by-step reproduction instructions

  1. Install Gutenberg 16.7.0 RC1
  2. change to theme Twenty Sixteen
  3. add page/post
  4. add image
  5. activate "expand on click"
  6. view page
  7. try lightbox-efect

Screenshots, screen recording, code snippet

No response

Environment info

WP 6.3.1
Gutenberg 16.7.0 RC1
Themes: Twenty Sixteen, GeneratePress

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@properlypurple properlypurple added [Type] Bug An existing feature does not function as intended [Block] Image Affects the Image Block labels Sep 21, 2023
@properlypurple
Copy link
Contributor

I've been able to reproduce this consistently with classic themes. In the block themes that I tested, the following css is applied (coming from here in Gutenberg )

.wp-lightbox-container button {
  background: none;
  border: none;
  cursor: zoom-in;
  height: 100%;
  position: absolute;
  width: 100%;
  z-index: 100;
}

@annezazu
Copy link
Contributor

cc @artemiomorales for your awareness!

@bph bph moved this from Triage to Needs Dev / Todo in WordPress 6.4 Editor Tasks Sep 23, 2023
@michalczaplinski
Copy link
Contributor

michalczaplinski commented Sep 26, 2023

@SantosGuillamot and I took a look at this together and I think we've got a fix. I'll send a PR shortly.

@t-hamano
Copy link
Contributor

t-hamano commented Oct 2, 2023

I believe this issue is the same as the issue reported in #54930.

@SantosGuillamot
Copy link
Contributor

I think you're right. I just dropped a comment there: link.

@afercia
Copy link
Contributor

afercia commented Oct 6, 2023

Hello everyone. I'm not sure the fix for the hover/focus implemented in #54837 is enough. Some theme nay use CSS for the hover / focus state that use selectors with a higher specificity. For example, testing with Twenty Seventeen, I still see the theme CSS for the buttons making the image button overlay completely grey on hover, which is the hover sstyle Twenty Seventeen uses for buttons:

Screenshot 2023-10-06 at 09 03 38

Looks like the two related CSS selectors have the same specificity but thte theme's stylesheet is enqueued later.

Lightbox: .wp-lightbox-container button:hover
Theme: :not( .mejs-button ) > button:hover

Seems to me the lightbox 'reset' rule needs a selector with much higher specificity.

See also #55097 for more considerations about overlaying the image with a transparent button.

Additionally:
There is no custom focus style provided for the Close button. With Twenty Twenty-Four, the focus style is the native browsers one. However, some themes reset the native focus style with outline: none and provide their own custom focus style. Most of the WordPress bundled themes do so. In this case, the Close button doesn't show any focus style. The lightbox CSS should make sure there is always a focus style.

@afercia afercia reopened this Oct 6, 2023
@artemiomorales artemiomorales moved this to Development Feedback in Image Lightbox Oct 11, 2023
@artemiomorales
Copy link
Contributor

The outstanding issue here may be resolved by #55212 depending on how conversations on design go. The hover style actually looks good when testing on this branch, so we can consider adding that style by default, or overriding it with a more detailed specificty there.

@artemiomorales artemiomorales moved this from Done to Needs Decision / Discussion in WordPress 6.4 Editor Tasks Oct 11, 2023
@artemiomorales
Copy link
Contributor

I just closed #54940, which is a duplicate of this.

There's a related Core ticket: https://core.trac.wordpress.org/ticket/59513

@bph bph moved this from Needs Decision / Discussion to Punted to 6.5 in WordPress 6.4 Editor Tasks Oct 13, 2023
@bph bph moved this from Punted to 6.5 to Needs Decision / Discussion in WordPress 6.4 Editor Tasks Oct 13, 2023
@bph bph moved this from Needs Decision / Discussion to Punted to 6.5 in WordPress 6.4 Editor Tasks Oct 13, 2023
@bph bph moved this from Punted to 6.5 to In Progress in WordPress 6.4 Editor Tasks Oct 13, 2023
@artemiomorales
Copy link
Contributor

This issue is largely fixed in #55212.

We may still run across some minor style inconsistencies with the :active or :hover styles (the :active style in Twenty Twelve changes the color of the button background slightly, for example — will hopefully be able to fix that soon), but as the button is in the top right corner now, I don't believe any of those inconsistencies would be showstoppers for the feature to be included in 6.4.

@annezazu
Copy link
Contributor

@artemiomorales can we close this out at this point and open up any particular remaining issues separately?

@artemiomorales
Copy link
Contributor

can we close this out at this point and open up any particular remaining issues separately?

@annezazu Yes! That sounds good to me.

@github-project-automation github-project-automation bot moved this from Dev Feedback to Done in Image Lightbox Oct 19, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in WordPress 6.4 Editor Tasks Oct 19, 2023
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 [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
No open projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants