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

docs: dashboard #814

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Conversation

ethanalvizo
Copy link
Contributor

@ethanalvizo ethanalvizo commented Sep 9, 2024

Closes #708

BREAKING CHANGE: Changed activeItemIdx to active_item_index in stack.py

@margaretkennedy
Copy link
Contributor

Do you plan to add more narrative and explanation of each of the sub-headings?

@ethanalvizo ethanalvizo marked this pull request as draft September 13, 2024 02:33
@ethanalvizo
Copy link
Contributor Author

Do you plan to add more narrative and explanation of each of the sub-headings?

Yes sorry, this was meant to be a draft pull request to show the potential layout examples. I'm not too familiar with the dashboard component myself so I was waiting on @dsmmcken for some thoughts on the copy

Copy link
Contributor

@dsmmcken dsmmcken left a comment

Choose a reason for hiding this comment

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

We need to cover recommendations (rules) and overall how it works, and that it is different then other components in some key ways;

Dashboards must be a child of the root script, and not nested inside a @ui.component. Otherwise the app is unable to determine the type of the component correctly.

dash = ui.dash(..)

not 

@ui.component
def my_dash ()...
  return ui.dashboard()

Introduce the key children that make up dashboards: row, column and stack

Dashboard must have one and only one child, which should typically be either a row or column.

Height/Width of panels are summed to 100%.

Examples should be more minimal. I would do them more like this:

my_dash = ui.dashboard(
            ui.row(
                ui.panel("A"),
                ui.panel("B")
            )
        )

And then have a variety examples showing different desired layouts:

  • 2 x 1
  • 1 x 2
  • 2x2 grid
  • 3 x 1 grid
  • split a row x 2
  • split a column x 2
  • basic stack
  • stack as part of a layout
  • non-even layouts like 60/40 layouts and stuff

Tracking state at the dashboard level

@ui.component
def layout():
    state, set_state ...
    return ui.row(
      ...
      )

dash = ui.dashboard(layout())

plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
Comment on lines 10 to 15
## Key Components
There are 3 main children that make up dashboard: row, column, and stack.

- **Row**: A container used to group elements horizontally. Each element is placed to the right of the previous one.
- **Column**: A container used to group elements vertically. Each element is placed below the previous one.
- **Stack**: A container used to group elements into tabs. Each element gets its own tab, with only one element visible at a time.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Panel should probably be mentioned here as well.

We should mention something about row/column being the "top" of the layout trees. Columns should go inside of rows and rows should go inside of columns when making a layout. You can put rows as children of rows and columns as children of columns, but it's basically a no-op I believe in terms of actual layout. Would need to double-check it doesn't do anything special. row(row(column, column), column) is the same visually as row(column, column, column)

Stacks/panels are the "bottom" of the tree. Once you add a stack or panel, the layout in that section is effectively done. If you put a column inside a row, you'll get a flex column within the panel. That detail probably isn't necessary for dashboard though as it is more a column/row/panel detail.

We also implicitly wrap the children when necessary, so you don't have to be explicit about the entire layout if you don't need the extra props of the wrappers.

If you have other row/column siblings, the node will be wrapped in an appropriate row/column. If there are no row/column children, then nodes will be wrapped in stacks if needed.

row(component, component) will become row(stack(component), stack(component)) while row(column(comp), comp) will become row(column(comp), column(comp)) and finally row(column(stack(comp)), column(stack(comp))). They will actually wrap the comp in a panel as well, but I left that out of my example.

The rules are basically as follows for automatic wrapping

  1. Dashboard - Wrap in row/column if no single node is the default (e.g. you can provide [col, col] as the child to dashboard)
  2. Row/column
    • If there are children that are rows/columns, wrap non-row/non-column children in row/column. Row elements wrap children in columns and vice-versa.
    • If there are no children that are rows/columns, wrap non-stack children in stacks
  3. Stack
    • Wrap non-panel children in panels

A final example on this. dashboard([t1, t2]) would become dashboard(column(stack(panel(t1)), stack(panel(t2)))). It would follow rule 1, then 2b, then 3.

Copy link
Contributor Author

@ethanalvizo ethanalvizo Sep 27, 2024

Choose a reason for hiding this comment

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

The row(row(column, column), column) actually impacts the width of the panels. The two children of the outer row are given the same width so this results in the inner row splitting 50% of the width in two for each of its inner columns while the column not in the inner row fills up the remaining 50% of the outer row.

So rather than AAA|BBB|CCC it's AA|BB|CCCC

from deephaven import ui

my_dash = ui.dashboard(ui.row(ui.row(ui.column("A"), ui.column("B")), ui.column("C")))

I do still think we should push users to put columns in rows and vice versa to make defining their layouts simpler but I don't think we should say it's visually the same thing. Unless that behaviour I discovered above is unintended^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried condensing these points into two main sections: Layout Hierarchy and Automatic Wrapping

Copy link
Contributor

Choose a reason for hiding this comment

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

AA|BB|CCCC

I've put rows in rows for that exact reason recently, so I assume that is both intended and useful?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is intentional AFAIK in golden-layout. It's just something we normally don't do when normalizing layouts because we set width/height instead

These 2 should be equivalent

row(row(col(comp), col(comp)), col(comp))
row(col(comp, width=25), col(comp, width=25), col(comp))

@ethanalvizo ethanalvizo marked this pull request as ready for review September 27, 2024 19:07
Copy link
Contributor

@dsmmcken dsmmcken left a comment

Choose a reason for hiding this comment

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

@mattrunyon @mofojed

Do you think we should have separate pages for row, column, stack and panel? or document those all here?

plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved


mw = ui.dashboard(multiwave())
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Add autodoc string.

Should we auto doc row, column, stack and panel all on this page? @mattrunyon @mofojed @jnumainville ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added but leaving unresolved so others can see comment

Copy link
Member

Choose a reason for hiding this comment

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

@dsmmcken I think we should document row/column/stack here, as it doesn't make sense outside of the dashboard context. Precedent would be similar to Radio in the Spectrum docs, as it doesn't make sense outside of RadioGroup so they document it there: https://react-spectrum.adobe.com/react-spectrum/RadioGroup.html#radio-props
I could see panel being separate since you can create a panel without a dashboard.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use row and column within a panel currently. They are basically an alias for flex row and flex column, but they don't have the other props of flex

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, panel gets it's own page, but row/col/stack can go under this.

## API Reference

```{eval-rst}
.. dhautofunction:: deephaven.ui.dashboard
```

### Row API Reference

```{eval-rst}
.. dhautofunction:: deephaven.ui.row
```

...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to mention in the flex page that row/column are aliases for flex(..., direction='row') and flex(..., direction='column') when used within a panel? You can't adjust things like flex-wrap or align-items, but they are meant to be simple versions of flex

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made note of this in the current flex docs PR to wait until this discussion is resolved

Copy link
Contributor Author

@ethanalvizo ethanalvizo Oct 15, 2024

Choose a reason for hiding this comment

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

Separate panel docs captured in #922

@ethanalvizo ethanalvizo mentioned this pull request Oct 4, 2024
Comment on lines 123 to 141
### Varying widths

```python
from deephaven import ui

dash_widths = ui.dashboard(
ui.row(ui.stack(ui.panel("A", title="A"), width=70), ui.panel("B", title="B"))
)
```

### Varying height

```python
from deephaven import ui

dash_heights = ui.dashboard(
ui.column(ui.stack(ui.panel("A", title="A"), height=70), ui.panel("B", title="B"))
)
```
Copy link
Contributor

Choose a reason for hiding this comment

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

This only shows setting height/width on a stacks. We should also note and show examples on settings height and width on row/column.

Rows can accept a height, and columns can accept a width.

Stacks that are a direct descendant of a column, can accept a height, and width if inside row. Setting the opposite dimension I believe is ignored.

That should be explained.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added. I pulled out the dimension changing examples from the Layout section into their own. I thought it would be too confusing including all of them in there

plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/dashboard.md Outdated Show resolved Hide resolved
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.

docs: ui.dashboard
6 participants