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

Chore: Safari fullscreen #930

Merged
merged 4 commits into from
Feb 15, 2019

Conversation

ConradJChan
Copy link
Contributor

@ConradJChan ConradJChan commented Feb 14, 2019

The previous attempt to fix this broke the adaptability of the .bp-content div from shrinking when the thumbnails sidebar was open.

In this fix, only when the .bp-is-fullscreen class is applied, do we set the width to 100% as well as set the background-color to inherit because Safari by default does not do this for flex items in a flex container that request fullscreen. In this way, it handles both the default #f5f5f5 as well as #000 when in a media viewer's "lowered lights" mode

@boxcla
Copy link

boxcla commented Feb 14, 2019

Verified that @ConradJChan has signed the CLA. Thanks for the pull request!

position: relative;
}

.bp-content.bp-is-fullscreen {
background-color: $black;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this exists today. Should it only apply to the media player?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you're right. Today it just keeps the preview background which is #f5f5f5 I think. But in Safari, the background changed to white. I'll update it with the #f5f5f5 value and make sure it's the same experience across browsers

@ConradJChan ConradJChan merged commit 794df00 into box:thumbnails-sidebar Feb 15, 2019
@ConradJChan ConradJChan deleted the safari-fullscreen branch February 15, 2019 21:48
ConradJChan pushed a commit to ConradJChan/box-content-preview that referenced this pull request Feb 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants