Skip to content

Commit

Permalink
Fullscreen Button Overflow Horizontally (streamlit#8279)
Browse files Browse the repository at this point in the history
## Describe your changes

As of now, our full screen button goes outside of the boundary, which in
some viewports can cause overflowing widths to make the container
scrollable. We intend to make the full screen button experience not as
bad, but for now, the solution is to reduce the content area size when
we hit this threshold.

## GitHub Issue Link (if applicable)
Closes streamlit#6990 

## Testing Plan

- E2E Test to capture the look created from the bug.

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
  • Loading branch information
kmcgrady authored Mar 18, 2024
1 parent fc25168 commit ea98bbe
Show file tree
Hide file tree
Showing 29 changed files with 122 additions and 43 deletions.
41 changes: 0 additions & 41 deletions e2e/specs/st_columns_layout.spec.js

This file was deleted.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 20 additions & 0 deletions e2e_playwright/responsive.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Copyright (c) Streamlit Inc. (2018-2022) Snowflake Inc. (2022-2024)
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import numpy as np

import streamlit as st

img = np.repeat(0, 3000 * 800).reshape(800, 3000)
st.image(img)
28 changes: 28 additions & 0 deletions e2e_playwright/responsive_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Copyright (c) Streamlit Inc. (2018-2022) Snowflake Inc. (2022-2024)
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from playwright.sync_api import Page, expect

from e2e_playwright.conftest import ImageCompareFunction


def test_fullscreen_button_edge_case(app: Page, assert_snapshot: ImageCompareFunction):
"""Test that window doesn't oveflow with window"""
app.set_viewport_size({"width": 780, "height": 400})
image = app.get_by_test_id("stFullScreenFrame")
expect(image).to_have_count(1)
image.hover()
expect(app.get_by_test_id("StyledFullScreenButton")).to_have_css("opacity", "1")

assert_snapshot(app, name="page_fullscreen_button_edge_case")
File renamed without changes.
49 changes: 49 additions & 0 deletions e2e_playwright/st_columns_layout_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Copyright (c) Streamlit Inc. (2018-2022) Snowflake Inc. (2022-2024)
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from playwright.sync_api import Page, expect

from e2e_playwright.conftest import ImageCompareFunction


def test_show_columns_horizontally_when_viewport_allows(
themed_app: Page, assert_snapshot: ImageCompareFunction
):
"""shows columns horizontally when viewport > 640"""
themed_app.set_viewport_size({"width": 641, "height": 800})
horizontal_blocks = themed_app.get_by_test_id("stHorizontalBlock")
expect(horizontal_blocks).to_have_count(2)

assert_snapshot(horizontal_blocks.nth(0), name="columns-layout-horizontal")


def test_show_columns_vertically_when_viewport_requires(
themed_app: Page, assert_snapshot: ImageCompareFunction
):
"""stacks columns vertically when viewport <= 640"""
themed_app.set_viewport_size({"width": 640, "height": 800})
horizontal_blocks = themed_app.get_by_test_id("stHorizontalBlock")
expect(horizontal_blocks).to_have_count(2)

assert_snapshot(horizontal_blocks.nth(0), name="columns-layout-vertical")


def test_columns_always_take_up_space(
themed_app: Page, assert_snapshot: ImageCompareFunction
):
"""columns still takes up space with no elements present"""
horizontal_blocks = themed_app.get_by_test_id("stHorizontalBlock")
expect(horizontal_blocks).to_have_count(2)

assert_snapshot(horizontal_blocks.nth(1), name="columns-with-one-element")
3 changes: 2 additions & 1 deletion frontend/app/src/components/AppView/AppView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ function AppView(props: AppViewProps): ReactElement {
toastAdjustment,
} = React.useContext(AppContext)

const { addScriptFinishedHandler, removeScriptFinishedHandler } =
const { addScriptFinishedHandler, removeScriptFinishedHandler, libConfig } =
React.useContext(LibContext)

const layout = wideMode ? "wide" : "narrow"
Expand Down Expand Up @@ -225,6 +225,7 @@ function AppView(props: AppViewProps): ReactElement {
hasBottom={hasBottomElements}
isEmbedded={embedded}
hasSidebar={showSidebar}
disableFullscreenMode={Boolean(libConfig.disableFullscreenMode)}
>
{renderBlock(elements.main)}
</StyledAppViewBlockContainer>
Expand Down
22 changes: 22 additions & 0 deletions frontend/app/src/components/AppView/styled-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export interface StyledAppViewBlockContainerProps {
isWideMode: boolean
showPadding: boolean
addPaddingForHeader: boolean
disableFullscreenMode: boolean
hasBottom: boolean
}

Expand All @@ -108,6 +109,7 @@ export const StyledAppViewBlockContainer =
isWideMode,
showPadding,
addPaddingForHeader,
disableFullscreenMode,
theme,
}) => {
let topEmbedPadding: string = showPadding ? "6rem" : "2.1rem"
Expand All @@ -120,6 +122,24 @@ export const StyledAppViewBlockContainer =
const bottomEmbedPadding =
showPadding && !hasBottom ? "10rem" : theme.spacing.lg
const wideSidePadding = isWideMode ? "5rem" : theme.spacing.lg

// Full screen-enabled elements can overflow the page when the screen
// size is slightly over the content max width.
// 50rem = contentMaxWidth + 2 * 2rem (size of button as margin)
// We use 0.5 to give a little extra space for a scrollbar that takes
// space like safari and avoid scrollbar jitter.
//
// See https://github.com/streamlit/streamlit/issues/6990
// TODO: Remove this workaround when we migrated to the new fullscreen buttons
const shouldHandleFullScreenButton =
!isWideMode && !disableFullscreenMode
const fullScreenButtonStyles = shouldHandleFullScreenButton
? {
[`@media (max-width: 50.5rem)`]: {
maxWidth: `calc(100vw - 4.5rem)`,
},
}
: {}
return {
width: theme.sizes.full,
paddingLeft: theme.spacing.lg,
Expand All @@ -134,6 +154,8 @@ export const StyledAppViewBlockContainer =
minWidth: isWideMode ? "auto" : undefined,
maxWidth: isWideMode ? "initial" : theme.sizes.contentMaxWidth,

...fullScreenButtonStyles,

[`@media print`]: {
minWidth: "100%",
paddingTop: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class FullScreenWrapper extends PureComponent<FullScreenWrapperProps, State> {
>
{!disableFullscreenMode && (
<StyledFullScreenButton
data-testid={"StyledFullScreenButton"}
data-testid="StyledFullScreenButton"
onClick={buttonOnClick}
title={buttonTitle}
isExpanded={expanded}
Expand Down

0 comments on commit ea98bbe

Please sign in to comment.