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

[charts][RFC] Replace DefaultXxxx by the usage of hooks #13819

Closed
alexfauquette opened this issue Jul 12, 2024 · 2 comments
Closed

[charts][RFC] Replace DefaultXxxx by the usage of hooks #13819

alexfauquette opened this issue Jul 12, 2024 · 2 comments
Labels
breaking change component: charts This is the name of the generic UI component, not the React module! RFC Request For Comments v8.x

Comments

@alexfauquette
Copy link
Member

alexfauquette commented Jul 12, 2024

Related messages

#13209 (comment)
#13700 (comment)

Issues that could be solved with it

Current state

The initial idea of slots was to provide developers with a simple interface to plug their custom components.

For example if they want to customize the tooltip, we provide all the data needed to the slots.tooltip and then they are responsible for rendering the content of the tooltip.

But the tooltip for an axis does not require the same data as a tooltip for an item. So either we have slots.tooltip = (props: AxisTooltipProps | ItemTooltipProps) => .... Or we creat two slots.

For now, we went with

function Tooltip() {
  // .. computation
  return <slots.popper>
    {isAxis ? <AxisTooltipContent /> : <ItemTooltipContent />}
  </slots.popper>
}

function AxisTooltipContent(){
  // .. computation specific to axis tooltip
  return <slots.axisTooltip />
}

function ItemTooltipContent(){
  // .. computation specific to item tooltip
  return <slots.itemTooltip />
}

Similar issue occurs for the legend due to the introduction of color legend (we have series legend, piecewise color legend, and continuous color legend)

Issue

This current solution seems to be contrary to the idea of supporting composition. Currently to have access to the processed data, a custom tooltip should be done as follows:

<ChartsContainer >
  <ChartsTooltip slots={{ itemTooltip: MyCustomTooltipContent }} />
</ChartsContainer>

Proposal

I propose to stop this pattern and use hooks instead.

function CustomAxisTooltip(){
  const tooltipProps = useChartsTooltip();
  const { series, axis, dataIndex, axisValue } = useChartsAxisTooltipContent();

  return <ChartsTooltip {...tooltipProps}>
    <Paper>
    {sereries.map((item) => {
      // render whatever you want
    })}
    </Paper>
  </ChartsTooltip>
}

Potential issues

Will need a nice docs for discoverability

Search keywords:

@alexfauquette
Copy link
Member Author

Closing since the Legend and Tooltip DevExp got reworked

Copy link

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: charts This is the name of the generic UI component, not the React module! RFC Request For Comments v8.x
Projects
None yet
Development

No branches or pull requests

1 participant