docs: list view #769
docs: list view #769
Conversation
This is what you're referring to right? @dsmmcken |
No, I mean ui.list_view pydoc string in the python source is incomplete. It's missing half its params. |
There was a problem hiding this comment.
@ethanalvizo The docs that are here look good overall (left some minor tweak comments), but I think we are missing some examples of different data sources. Namely we should have both static items + table backed examples. Looking at the picker docs, I also see a "UI Recommendations" section. @dsmmcken am I remembering correctly that this is something you requested for docs for each component?
Made this the base example. I stripped down the event examples to be much simpler but I chose to still use table sources for the items in the list view. I think having keys makes for better examples of the possible actions and they're needed for the disabled options section |
|
|
||
| ## Table Source Example | ||
|
|
||
| List view items can also be generated from a table. |
There was a problem hiding this comment.
Should explain what ui.item_table_source is, and why it would be needed.
Should show an example both passing a table directly to listview and a item_table_source with keys and label.
There was a problem hiding this comment.
An item table source wraps a Table or PartitionedTable to provide additional information for creating complex items from a table.
I couldn't really gather what a "complex item" is from the pydocs of item_table_source^. I know if you don't use table source it will just use the first column as the key and label and I made note of it but other than that I wasn't sure what the difference is.
| aria_label="List View - Quiet", | ||
| on_change=set_value, | ||
| selected_keys=value, | ||
| overflow_mode="wrap", |
There was a problem hiding this comment.
@bmingles does that work with item table sources? I don't think it should right, because they need to be equal row heights? Are we doing anything to prevent that?
There was a problem hiding this comment.
This is not supported. Variable item heights will cause bad behavior.
There was a problem hiding this comment.
Wasn't sure if this meant I should take the example out entirely but I instead added the note below explaining that it is not currently supported. Not sure if I should add the details of what the bad behavior is, if so is there an easy example I could showcase @bmingles?
Note: Currently not supported if a table source is used.
There was a problem hiding this comment.
The example is fine for static source. The issue is only with table sources since the listview has to be able to calculate the total content height. We can do this if all items have the same height, since it's just multiplying item height * table size. Variable heights break this.
There was a problem hiding this comment.
Changed the note to say
Note: Currently not supported if a table source is used and items have varying heights.
There was a problem hiding this comment.
I left that suggestion in a note, but @bmingles indicates it's not entirely accurate. I resolved my comment.
| ```{eval-rst} | ||
| .. dhautofunction:: deephaven.ui.list_view | ||
| ``` |
There was a problem hiding this comment.
Add autodoc for deephaven.ui.item_table_source here.
Side note: Spectrum ListView docs do not add the Item props, they just have a specific section for it under Complex Items.
I think we should at least autodoc ui.item_table_source here, maybe don't need to autodoc ui.item as long as we have a "complex items" section showing adding actions to a listview with just items (we already have an example from a table source with actions, so just a list of items with actions is needed).
There was a problem hiding this comment.
When I pass in an actions prop to a static list view the options are not rendering. Not sure if the issue is with my implementation or actions are not supported for non-table sources
lv = ui.list_view(
"Option 1",
"Option 2",
"Option 3",
actions = ui.list_action_group(
"Edit",
"Delete",
on_action=print("hello"),
)
)
There was a problem hiding this comment.
Spoke to Brian about this and it is not supported when the children are coming from a non-table source. The actiongroup functionality is added by useRenderNormalizedIem which only runs in table sourced list view.
It's not always data being passed in it could be JSX elements as children. Using the renderItem function to add actions would involve wrapping JSX elements + creating new element trees. It's possible but it was simpler to just let it pass so right now there is no opportunity to inject the new elements required for action groups
| default_selected_keys: The initial selected keys in the collection (uncontrolled). | ||
| selection_style: How the selection should be displayed. | ||
| on_action: Handler that is called when the user performs an action on an item. The user event depends on the collection's selection_style and interaction modality. | ||
| on_selection_change: Handler that is called when the selection changes. |
There was a problem hiding this comment.
@dsmmcken trying to remember. Do we actually want to surface on_selection_change since we are encouraging on_change? We marked as deprecated in some places, but I think we wanted to remove it from new apis.
There was a problem hiding this comment.
I am not sure one what our general pattern of deprecation is, or should be. But yes the on_change handler is the preferred one to show in examples, and on_selection_change can eventually be removed I think?
There was a problem hiding this comment.
Changed the examples and updated the pydocs to reflect the preference. Wasn't certain about fully omitting on_selection_change though
Closes #686
Note: Empty state example was intentionally omitted as this bug was found when rendering an empty list view component. Can add in the example once that is fixed