[Horiz Layout] Columns by akrolsmir · Pull Request #1979 · streamlit/streamlit

akrolsmir · 2020-09-08T19:56:51Z

akrolsmir · 2020-09-15T21:41:05Z

frontend/src/lib/DeltaParser.ts

-      : List()
-
-  return ImmutableMap({ element: list, reportId, metadata, deltaBlock })
+  const existingEl = reportElement?.get("element")


Note: this is supposed to be a pure refactor, no functionality change.

akrolsmir · 2020-09-15T21:42:29Z

lib/streamlit/delta_generator.py

@@ -426,12 +449,7 @@ def expander(self, label=None, expanded=False):
        return self._block(block_proto=block_proto)

    def favicon(


I really don't know why this keeps flopping back and forth. In any case, favicon() is an unused method; I'll kill it soonish

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining

karriebear · 2020-09-16T06:38:35Z

frontend/src/components/core/Block/Block.tsx

        const element = reportElement.get("element")

        if (element instanceof List) {
+          // Recursive case AKA a single container AKA node with children


A lot of AKAs lol

karriebear · 2020-09-16T06:49:36Z

lib/streamlit/delta_generator.py

+            return col_proto
+
+        horiz_proto = Block_pb2.Block()
+        horiz_proto.horizontal.unused = False


This unused name confused me. When I first read this, I assumed you were reclassifying columns to be a vertical layout which puzzled me. But I guess you just needed a dummy name to check if it exists?

Yeah, needed to set something so that I could check against the presence of the Horizontal() proto later.

Set it to horizontal.unused = True, which I think makes a bit more sense.

karriebear · 2020-09-16T06:56:15Z

frontend/src/components/core/Block/Block.tsx

+      // Create a horizontal block as the parent for columns
+      // TODO: Calculate widths for children? We just pick a big number for now.
+      return (
+        <div className="stBlock-horiz" style={{ display: "flex" }}>


The style tag is a bit ugly. Doesn't have to be part of this PR but would be great to have this in a styled component.

That's a good point, I'll add a task to go through and clean up these up.

karriebear · 2020-09-16T07:04:36Z

frontend/src/components/core/Block/Block.tsx

+      // TODO: Calculate widths for children? We just pick a big number for now.
+      return (
+        <div className="stBlock-horiz" style={{ display: "flex" }}>
+          {this.renderElements(8888)}


I was going to try this out but at this point, I'm just going to ask... but is width needed for horizontal block if we're going to flex it? From what i'm getting this renderElements will be just for the block and the children will end up running through the whole thing and hitting the autosizer below?

Huh, you're absolutely right, actually; hadn't realized that was the case. Setting to "0" as an ignorable parameter, for now.

karriebear · 2020-09-16T07:05:12Z

frontend/src/components/core/Block/Block.tsx

-  )
+  public render = (): ReactNode => {
+    if (this.props.deltaBlock?.horizontal) {
+      // Create a horizontal block as the parent for columns


Accurate for now but it'd be great to have horizontal blocks in the future that are not necessarily housing just columns

karriebear · 2020-09-16T22:51:18Z

frontend/src/components/core/Block/Block.tsx

+      // Create a horizontal block as the parent for columns
+      // TODO: Calculate widths for children? We just pick a big number for now.
+      return (
+        <div className="stBlock-horiz" style={{ display: "flex", gap: "8px" }}>


Let's style this as a component too! Definitely a cleanup activity that can wait for another PR

* [Horizontal Layout] Implement st.container() (#1961) * Fix up dg._block() to be dg.container() * Add a unit test * Add an e2e test for st.container * Minor cleanups * Clean up variable names and comments * Clean up deltaPath code and docs * [Horizontal Layout] Implement `with DG` (#1964) * Force reinstall of setuptools to 49.6.0 (#1962) * try updating in virtualenv * new cache * am i using 39.0.1 and if so why is it failing? * somewhere somehow we are upgrading to setuptools 50.0.3... but where..? * looks like somewhere between setup + pipenv-install we get a newer version of setuptools * looks like somewhere between setup + pipenv-install we get a newer version of setuptools * force reinstall * looks like it's test requirements messing us up? * force reinstall + cleanup debugs * Fix up dg._block() to be dg.container() * Add a unit test * Add an e2e test for st.container * Minor cleanups * Clean up variable names and comments * Clean up deltaPath code and docs * Organize DGs in a tree, to track the `with DG` * Enable `with` in every st.foo call * Appease mypy with # type: ignore * More sacrifices at the altar of mypy * Add unit and e2e tests for with * Actually, don't rely on each DG mixin to call _active_dg Revert "Appease mypy with # type: ignore" This reverts commit 4ba7c47. Revert "Enable `with` in every st.foo call" This reverts commit 9b467d6. * Use the active DG for all "protected" class methods * Clean up DG docs * Don't switch DGs from add_rows Per Karrie's suggestion Co-authored-by: karrie <[email protected]> * [horiz-layout] Add collapsible_container (#1978) * Add collapsible_container * Add default collapsed * make label required + initial docs * Consolidate with WIP column work * Address PR comments for more cleanliness * Update e2e tests * Rename collapsible container (#1993) * rename to expander * Fix tests * Rename collapsible to expandable * [Horiz Layout] Columns (#1979) * Implement columns with variable widths * Add a e2e test for columns * Add a unit test * Update Prettier to 1.19 to allow optional chaining https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining * Fix e2e test * Downgrade Prettier back to 1.18 * Empty columns now take up space * Remove duplicate function from merge conflict * Use flex-gap instead of margin, so column sides don't get padded * Minor tweaks requested by code review * Prefix new layout methods with `beta_` Co-authored-by: karrie <[email protected]>

nthmost mentioned this pull request Sep 9, 2020

2nd Sidebar (on right side) #1980

Open

akrolsmir force-pushed the columns branch from dff8633 to 87621a4 Compare September 15, 2020 19:59

akrolsmir changed the title ~~[WIP] [Horiz Layout] Columns~~ [Horiz Layout] Columns Sep 15, 2020

akrolsmir marked this pull request as ready for review September 15, 2020 20:00

akrolsmir requested a review from a team September 15, 2020 20:00

Implement columns with variable widths

30d97d1

akrolsmir force-pushed the columns branch from 87621a4 to 30d97d1 Compare September 15, 2020 21:18

Add a e2e test for columns

e792975

akrolsmir commented Sep 15, 2020

View reviewed changes

akrolsmir added 7 commits September 15, 2020 15:07

Add a unit test

04372d8

Update Prettier to 1.19 to allow optional chaining

1c4451a

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining

Fix e2e test

aeeb01f

Downgrade Prettier back to 1.18

06a514e

Empty columns now take up space

d01be3f

Remove duplicate function from merge conflict

c49a4b1

Use flex-gap instead of margin, so column sides don't get padded

28a9de7

karriebear approved these changes Sep 16, 2020

View reviewed changes

Minor tweaks requested by code review

15cbb18

akrolsmir merged commit 16f2794 into streamlit:horiz-layout Sep 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Horiz Layout] Columns#1979

[Horiz Layout] Columns#1979
akrolsmir merged 10 commits intostreamlit:horiz-layoutfrom
akrolsmir:columns

akrolsmir commented Sep 8, 2020 •

edited

Loading

Uh oh!

akrolsmir Sep 15, 2020

Uh oh!

akrolsmir Sep 15, 2020

Uh oh!

karriebear Sep 16, 2020

Uh oh!

karriebear Sep 16, 2020

Uh oh!

akrolsmir Sep 17, 2020

Uh oh!

karriebear Sep 16, 2020

Uh oh!

akrolsmir Sep 17, 2020

Uh oh!

karriebear Sep 16, 2020

Uh oh!

akrolsmir Sep 17, 2020

Uh oh!

karriebear Sep 16, 2020

Uh oh!

karriebear Sep 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -426,12 +449,7 @@ def expander(self, label=None, expanded=False):
		return self._block(block_proto=block_proto)

		def favicon(

Conversation

akrolsmir commented Sep 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

akrolsmir commented Sep 8, 2020 •

edited

Loading