feat: Pie: support multiple activeIds
#2670
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -20,6 +20,9 @@ import { | |||
| CommonPieProps, | |||
| } from './types' | |||
|
|
|||
| const idIsActive = (id: DatumId, activeId: null | DatumId | DatumId[]) => | |||
There was a problem hiding this comment.
Rather than supporting a scalar or an array, I think it would be better to rename the property to activeIds and to support arrays exclusively.
It would also be nice to add tests about this new feature and to adjust the documentation.
There was a problem hiding this comment.
I wanted to avoid making this a breaking change to anyone using activeId. Do you think it's worth it?
There was a problem hiding this comment.
I understand, but it makes the code and typing more complex, I'd prefer to have something a bit simpler to maintain, also the name of the property is now confusing IMHO.
| @@ -282,6 +282,76 @@ export const ControlledActiveId: Story = { | |||
| render: () => <ControlledPies />, | |||
| } | |||
|
|
|||
| const ControlledPiesMultipleActiveIds = () => { | |||
| const [activeId, setActiveId] = useState<string>(commonProperties.data[1].id) | |||
| const [additionalActiveIds, setAdditionalActiveIds] = useState<string>([]) | |||
There was a problem hiding this comment.
The type should be string[] I think.
No description provided.