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

Fullscreen mode not working in image viewer for essay and splash pages. #849

Open
1 task done
Erin-Cecele opened this issue Sep 14, 2023 · 2 comments
Open
1 task done
Assignees
Labels
bug Actual behavior does not match expected behavior quire-11ty This is an issue with the quire-11ty package status:triage needed Issue needs to be triaged by the Quire team. This label is automatically applied to new issues.

Comments

@Erin-Cecele
Copy link
Collaborator

Erin-Cecele commented Sep 14, 2023

Before proceeding, make sure there isn’t an existing issue for this bug.

  • I have searched the existing issues and determined this is a new bug.

Expected Behavior

No matter the layout type, when an image is opened in the image viewer, and you click the fullscreen button in the upper left corner, it should open the image in fullscreen mode.

Actual Behavior

On the essay and splash pages, when the image is opened in the viewer and the fullscreen button is pushed, nothing happens. The image does not go fullscreen.

Important to note: you can open images in fullscreen mode on the entry type pages.

Steps to Reproduce

  1. Start a new quire project
  2. Navigate to any of the images on the Introduction (splash) or American Photographs (essay) pages, open the images in the image viewer and click the fullscreen button

Version Numbers

[test-project]
quire-cli 1.0.0-rc.10
quire-11ty 1.0.0-rc.14
starter https://github.com/thegetty/[email protected]
[System]
quire-cli 1.0.0-rc.10
node v18.16.0
npm 9.5.1
os Darwin 21.6.0

Web Browser

I tested with Chrome 114 and Firefox 117, and @cbutcosk tested on Safari 16.6 and Chrome 116.

Relevant Terminal/Shell Output

No response

Supporting Information

Special thank you to @cbutcosk for bringing this bug to our attention in #815. As noted by @cbutcosk "I suspect your developers also need to add a Shadow DOM polyfill for Safari < 16.5 (mobile and desktop), which in this issue breaks in a slightly different way from the Chromium-based browsers."

So when fixing this issue be sure to test in multiple browsers!

@Erin-Cecele Erin-Cecele added the status:triage needed Issue needs to be triaged by the Quire team. This label is automatically applied to new issues. label Sep 14, 2023
@Erin-Cecele Erin-Cecele self-assigned this Sep 14, 2023
@Erin-Cecele Erin-Cecele mentioned this issue Sep 14, 2023
18 tasks
@Erin-Cecele Erin-Cecele added quire-11ty This is an issue with the quire-11ty package bug Actual behavior does not match expected behavior labels Oct 11, 2023
@cbutcosk
Copy link
Collaborator

I had a chance to peer into this, it turns out this line is the culprit on browsers that should otherwise support the fullscreen API:

return document.fullscreen || this.wrapper.classList.contains('quire-entry__lightbox--fullscreen')

Optional-chaining the wrapper and classList here lets the function succeed and open fullscreen as you'd expect. I didn't have a shadowDOM-unsupporting browser available but fullscreen did still work when I disabled shadowDOM on Safari 17.0 for whatever that is worth.

@Erin-Cecele
Copy link
Collaborator Author

Hi @cbutcosk, Thanks for finding the root cause of this issue. Do you have a fix you could possibly submit as a PR? You can find instructions via our Contributing Guidelines: https://github.com/thegetty/quire/blob/main/CONTRIBUTING.md#submit-your-contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Actual behavior does not match expected behavior quire-11ty This is an issue with the quire-11ty package status:triage needed Issue needs to be triaged by the Quire team. This label is automatically applied to new issues.
Projects
None yet
Development

No branches or pull requests

2 participants