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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gap param to st.columns #4887

Merged
merged 9 commits into from Jul 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions e2e/scripts/st_columns.py
Expand Up @@ -29,3 +29,19 @@
# Variable-width columns
for c in st.columns((1, 2, 3, 4)):
c.image(CAT_IMAGE)

# Various column gaps
c4, c5, c6 = st.columns(3, gap="small")
c4.image(CAT_IMAGE)
c5.image(CAT_IMAGE)
c6.image(CAT_IMAGE)

c7, c8, c9 = st.columns(3, gap="medium")
c7.image(CAT_IMAGE)
c8.image(CAT_IMAGE)
c9.image(CAT_IMAGE)

c10, c11, c12 = st.columns(3, gap="large")
c10.image(CAT_IMAGE)
c11.image(CAT_IMAGE)
c12.image(CAT_IMAGE)
18 changes: 17 additions & 1 deletion e2e/specs/st_columns.spec.js
Expand Up @@ -61,7 +61,7 @@ describe("st.column", () => {
// This assertion ensures that the report rerun completes first
cy.get("[data-testid='stHorizontalBlock'] [data-testid='column']").should(
"have.length",
7
16
);

// When layout was shifting, there was an old "flex: 8" block here.
Expand All @@ -70,4 +70,20 @@ describe("st.column", () => {
3
).should("have.css", "flex", "1 1 calc(10% - 16px)");
});

it("creates small gap between columns", () => {
cy.getIndexed("[data-testid='stHorizontalBlock']",
2).matchThemedSnapshots("columns-small-gap");
});

it("creates medium gap between columns", () => {
cy.getIndexed("[data-testid='stHorizontalBlock']",
3).matchThemedSnapshots("columns-medium-gap");
});

it("creates large gap between columns", () => {
cy.getIndexed("[data-testid='stHorizontalBlock']",
4).matchThemedSnapshots("columns-large-gap");
});

});
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion frontend/src/components/core/Block/Block.test.tsx
Expand Up @@ -35,7 +35,7 @@ function makeHorizontalBlock(numColumns: number): BlockNode {

return new BlockNode(
Array.from({ length: numColumns }, () => makeColumn(weight)),
new BlockProto({ allowEmpty: true, horizontal: true })
new BlockProto({ allowEmpty: true, horizontal: { gap: "small" } })
)
}

Expand Down
5 changes: 4 additions & 1 deletion frontend/src/components/core/Block/Block.tsx
Expand Up @@ -105,6 +105,7 @@ const BlockNodeRenderer = (props: BlockPropsWithWidth): ReactElement => {
return (
<StyledColumn
weight={node.deltaBlock.column.weight ?? 0}
gap={node.deltaBlock.column.gap ?? ""}
data-testid="column"
>
{child}
Expand Down Expand Up @@ -173,8 +174,10 @@ const HorizontalBlock = (props: BlockPropsWithWidth): ReactElement => {
// Create a horizontal block as the parent for columns.
// The children are always columns, but this is not checked. We just trust the Python side to
// do the right thing, then we ask ChildRenderer to handle it.
const gap = props.node.deltaBlock.horizontal?.gap ?? ""

return (
<StyledHorizontalBlock data-testid="stHorizontalBlock">
<StyledHorizontalBlock gap={gap} data-testid="stHorizontalBlock">
<ChildRenderer {...props} />
</StyledHorizontalBlock>
)
Expand Down
45 changes: 33 additions & 12 deletions frontend/src/components/core/Block/styled-components.ts
Expand Up @@ -19,16 +19,35 @@ import React from "react"
import styled from "@emotion/styled"
import { Theme } from "src/theme"

export const StyledHorizontalBlock = styled.div(({ theme }) => ({
// While using flex for columns, padding is used for large screens and gap
// for small ones. This can be adjusted once more information is passed.
// More information and discussions can be found: Issue #2716, PR #2811
display: "flex",
flexWrap: "wrap",
flexGrow: 1,
alignItems: "stretch",
gap: theme.spacing.lg,
}))
function translateGapWidth(gap: string, theme: Theme): string {
let gapWidth = theme.spacing.lg
if (gap === "medium") {
gapWidth = theme.spacing.threeXL
} else if (gap === "large") {
gapWidth = theme.spacing.fourXL
}
return gapWidth
}
export interface StyledHorizontalBlockProps {
gap: string
}

export const StyledHorizontalBlock = styled.div<StyledHorizontalBlockProps>(
({ theme, gap }) => {
const gapWidth = translateGapWidth(gap, theme)

return {
// While using flex for columns, padding is used for large screens and gap
// for small ones. This can be adjusted once more information is passed.
// More information and discussions can be found: Issue #2716, PR #2811
display: "flex",
flexWrap: "wrap",
flexGrow: 1,
alignItems: "stretch",
gap: gapWidth,
}
}
)

export interface StyledElementContainerProps {
isStale: boolean
Expand Down Expand Up @@ -74,12 +93,14 @@ export const StyledElementContainer = styled.div<StyledElementContainerProps>(

interface StyledColumnProps {
weight: number
gap: string
}

export const StyledColumn = styled.div<StyledColumnProps>(
({ weight, theme }) => {
({ weight, gap, theme }) => {
const percentage = weight * 100
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: both the HorizontalBlock (parent element) and each of the StyledColumns need to be aware of the specified gap

const width = `calc(${percentage}% - ${theme.spacing.lg})`
const gapWidth = translateGapWidth(gap, theme)
const width = `calc(${percentage}% - ${gapWidth})`

return {
// Calculate width based on percentage, but fill all available space,
Expand Down
Expand Up @@ -383,6 +383,7 @@ exports[`Tooltip element renders running img correctly with custom dark backgrou
"sidebarTopSpace": "6rem",
},
"spacing": Object {
"fourXL": "4rem",
"halfSmFont": "7px",
"lg": "1rem",
"md": "0.75rem",
Expand Down Expand Up @@ -795,6 +796,7 @@ exports[`Tooltip element renders running img correctly with custom dark backgrou
"sidebarTopSpace": "6rem",
},
"spacing": Object {
"fourXL": "4rem",
"halfSmFont": "7px",
"lg": "1rem",
"md": "0.75rem",
Expand Down Expand Up @@ -3554,6 +3556,7 @@ exports[`Tooltip element renders running img correctly with custom light backgro
"sidebarTopSpace": "6rem",
},
"spacing": Object {
"fourXL": "4rem",
"halfSmFont": "7px",
"lg": "1rem",
"md": "0.75rem",
Expand Down Expand Up @@ -3966,6 +3969,7 @@ exports[`Tooltip element renders running img correctly with custom light backgro
"sidebarTopSpace": "6rem",
},
"spacing": Object {
"fourXL": "4rem",
"halfSmFont": "7px",
"lg": "1rem",
"md": "0.75rem",
Expand Down Expand Up @@ -6725,6 +6729,7 @@ exports[`Tooltip element renders running img correctly with darkTheme 1`] = `
"sidebarTopSpace": "6rem",
},
"spacing": Object {
"fourXL": "4rem",
"halfSmFont": "7px",
"lg": "1rem",
"md": "0.75rem",
Expand Down Expand Up @@ -7137,6 +7142,7 @@ exports[`Tooltip element renders running img correctly with darkTheme 1`] = `
"sidebarTopSpace": "6rem",
},
"spacing": Object {
"fourXL": "4rem",
"halfSmFont": "7px",
"lg": "1rem",
"md": "0.75rem",
Expand Down Expand Up @@ -9896,6 +9902,7 @@ exports[`Tooltip element renders running img correctly with lightTheme 1`] = `
"sidebarTopSpace": "6rem",
},
"spacing": Object {
"fourXL": "4rem",
"halfSmFont": "7px",
"lg": "1rem",
"md": "0.75rem",
Expand Down Expand Up @@ -10308,6 +10315,7 @@ exports[`Tooltip element renders running img correctly with lightTheme 1`] = `
"sidebarTopSpace": "6rem",
},
"spacing": Object {
"fourXL": "4rem",
"halfSmFont": "7px",
"lg": "1rem",
"md": "0.75rem",
Expand Down
1 change: 1 addition & 0 deletions frontend/src/theme/primitives/spacing.ts
Expand Up @@ -30,4 +30,5 @@ export const spacing = {
xl: "1.25rem",
twoXL: "1.5rem",
threeXL: "2rem",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wasn't sure if I should add the new rem specification for large gap (4 rem) to the theme's spacing object... Can remove this if its unnecessary.

fourXL: "4rem",
}
28 changes: 25 additions & 3 deletions lib/streamlit/elements/layouts.py
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import cast, List, Sequence, TYPE_CHECKING, Union
from typing import cast, List, Sequence, TYPE_CHECKING, Union, Optional

from streamlit.beta_util import function_beta_warning
from streamlit.errors import StreamlitAPIException
Expand Down Expand Up @@ -69,7 +69,9 @@ def container(self) -> "DeltaGenerator":
return self.dg._block()

# TODO: Enforce that columns are not nested or in Sidebar
def columns(self, spec: SpecType) -> List["DeltaGenerator"]:
def columns(
self, spec: SpecType, *, gap: Optional[str] = "small"
) -> List["DeltaGenerator"]:
"""Insert containers laid out as side-by-side columns.

Inserts a number of multi-element containers laid out side-by-side and
Expand Down Expand Up @@ -97,6 +99,10 @@ def columns(self, spec: SpecType) -> List["DeltaGenerator"]:
For example, `st.columns([3, 1, 2])` creates 3 columns where
the first column is 3 times the width of the second, and the last
column is 2 times that width.
gap : string ("small", "medium", or "large")
An optional string, which indicates the size of the gap between each column.
The default is a small gap between columns. This argument can only be supplied by
keyword.

Returns
-------
Expand Down Expand Up @@ -159,14 +165,30 @@ def columns(self, spec: SpecType) -> List["DeltaGenerator"]:
if len(weights) == 0 or any(weight <= 0 for weight in weights):
raise weights_exception

def column_gap(gap):
if type(gap) == str:
gap_size = gap.lower()
valid_sizes = ["small", "medium", "large"]

if gap_size in valid_sizes:
return gap_size

raise StreamlitAPIException(
'The gap argument to st.columns must be "small", "medium", or "large". \n'
f"The argument passed was {gap}."
)

gap_size = column_gap(gap)

def column_proto(normalized_weight: float) -> BlockProto:
col_proto = BlockProto()
col_proto.column.weight = normalized_weight
col_proto.column.gap = gap_size
col_proto.allow_empty = True
return col_proto

block_proto = BlockProto()
block_proto.horizontal.SetInParent()
block_proto.horizontal.gap = gap_size
row = self.dg._block(block_proto)
total_weight = sum(weights)
return [row._block(column_proto(w / total_weight)) for w in weights]
Expand Down
51 changes: 51 additions & 0 deletions lib/tests/streamlit/layouts_test.py
Expand Up @@ -66,6 +66,57 @@ def test_not_equal_width_float_columns(self):
self.assertEqual(columns_blocks[1].add_block.column.weight, 2.5 / sum_weights)
self.assertEqual(columns_blocks[2].add_block.column.weight, 5.0 / sum_weights)

def test_columns_with_default_small_gap(self):
"""Test that it works correctly with no gap argument (gap size is default of small)"""

columns = st.columns(3)

all_deltas = self.get_all_deltas_from_queue()

horizontal_block = all_deltas[0]
columns_blocks = all_deltas[1:4]

# 4 elements will be created: 1 horizontal block, 3 columns, each receives "small" gap arg
self.assertEqual(len(all_deltas), 4)
self.assertEqual(horizontal_block.add_block.horizontal.gap, "small")
self.assertEqual(columns_blocks[0].add_block.column.gap, "small")
self.assertEqual(columns_blocks[1].add_block.column.gap, "small")
self.assertEqual(columns_blocks[2].add_block.column.gap, "small")

def test_columns_with_medium_gap(self):
"""Test that it works correctly with "medium" gap argument"""

columns = st.columns(3, gap="medium")

all_deltas = self.get_all_deltas_from_queue()

horizontal_block = all_deltas[0]
columns_blocks = all_deltas[1:4]

# 4 elements will be created: 1 horizontal block, 3 columns, each receives "medium" gap arg
self.assertEqual(len(all_deltas), 4)
self.assertEqual(horizontal_block.add_block.horizontal.gap, "medium")
self.assertEqual(columns_blocks[0].add_block.column.gap, "medium")
self.assertEqual(columns_blocks[1].add_block.column.gap, "medium")
self.assertEqual(columns_blocks[2].add_block.column.gap, "medium")

def test_columns_with_large_gap(self):
"""Test that it works correctly with "large" gap argument"""

columns = st.columns(3, gap="LARGE")

all_deltas = self.get_all_deltas_from_queue()

horizontal_block = all_deltas[0]
columns_blocks = all_deltas[1:4]

# 4 elements will be created: 1 horizontal block, 3 columns, each receives "large" gap arg
self.assertEqual(len(all_deltas), 4)
self.assertEqual(horizontal_block.add_block.horizontal.gap, "large")
self.assertEqual(columns_blocks[0].add_block.column.gap, "large")
self.assertEqual(columns_blocks[1].add_block.column.gap, "large")
self.assertEqual(columns_blocks[2].add_block.column.gap, "large")


class ExpanderTest(testutil.DeltaGeneratorTestCase):
def test_label_required(self):
Expand Down
2 changes: 2 additions & 0 deletions proto/streamlit/proto/Block.proto
Expand Up @@ -31,10 +31,12 @@ message Block {
}

message Horizontal {
string gap = 1;
}

message Column {
double weight = 1;
string gap = 2;
}

message Expandable {
Expand Down