feat: ui.menu component #1076
feat: ui.menu component #1076
Conversation
Co-authored-by: Mike Bender <[email protected]>
|
|
||
| The `is_open` and `default_open` props on the `menu_trigger` control whether the Menu is open by default. They apply controlled and uncontrolled behavior on the `menu` respectively. | ||
|
|
||
| ```python |
There was a problem hiding this comment.
I see this example was copied from the Spectrum docs, but it doesn't really seem to showcase what the props do since it behaves the same as if is_open wasn't set at all.
There was a problem hiding this comment.
Added a text to show the state.
There was a problem hiding this comment.
Since the description says the props control whether the Menu is open by default, it might be good to initialize the state with ui.use_boolean(True) so that it is open initially.
|
|
||
| from deephaven.table import Table, PartitionedTable | ||
| from .basic import component_element | ||
| from .section import SectionElement, Item |
There was a problem hiding this comment.
There seem to be a number of unused imports in this module.
| default_selected_keys: SelectionAll | List[Key] | None = None, | ||
| on_action: Callable[[Key], None] | None = None, | ||
| on_close: Callable[[], None] | None = None, | ||
| on_selection_change: Callable[[SelectionAll | List[Key]], None] | None = None, |
There was a problem hiding this comment.
For a lot of dh.ui components we included a conventional on_change prop and deprecated the on_selection_change or comparable prop. Not sure if we decided not to do that for menu. Just calling it out in case we missed it.
| callback(value); | ||
| return; | ||
| } | ||
| callback([...value] as [Key]); |
There was a problem hiding this comment.
The [Key] type suggests a tuple containing a single item. Can ...value contain multiple?
There was a problem hiding this comment.
This was a typescript typo elsewhere. I had defined [Key] instead of Key[].
| return <DHCMenu {...menuProps} />; | ||
| } | ||
|
|
||
| Menu.displayName = 'Menu'; |
There was a problem hiding this comment.
No harm in having this here, but I don't think displayName is needed on plain function components since dev tools can derive it from the function name. It is still needed on arrow functions.
Co-authored-by: Brian Ingles <[email protected]>
plugins/ui/docs/components/menu.md
Outdated
|
|
||
| ## Content | ||
|
|
||
| Menu accepts `item` elements as children, each with a `key` prop. Basic usage of `menu`, seen in the example above, shows multiple items populated with a string. |
There was a problem hiding this comment.
When you say "Menu" do you really mean menu? Or "Menus accept / A menu accepts"
There was a problem hiding this comment.
Change to:
A menu accepts
Co-authored-by: margaretkennedy <[email protected]>
ui.menucomponent fordeephaven.uihttps://deephaven.atlassian.net/browse/DH-18089
Python, Javascript, and docs.