zakodium-oss / react-science

React components and tools to build scientific applications.
https://react-science.pages.dev
MIT License
3 stars 6 forks source link

Review Toolbar.Item and Toolbar.PopoverItem #704

Open stropitek opened 3 months ago

stropitek commented 3 months ago

We should check which props we want to expose for each of those 2 components.

Right now we are exposing too many props and we should have an intermediate private component which exposes those.

Example: isPopover should not be exposed.

hamed-musallam commented 3 months ago

We shouldn't expose isPopover, and popoverInteractionKind is unnecessary for Toolbar.Item. It should be moved to the Toolbar.PopoverItem level.

stropitek commented 3 months ago

popoverInteractionKind is unnecessary for Toolbar.Item

it is on the toolbar and that is fine IMO. I don't think a toolbar should mix different interaction kind together. It should be documented that it only affects the popover items.