Feat/box plot labels #17579
Feat/box plot labels #17579
Conversation
|
Thanks for your contribution! |
|
I am happy to move the |
|
Can you provide an image to illustrate what this PR is about? |
|
@Ovilia Sure - I added it to the description. |
|
@Ovilia What do you mean by alignment of the labels? |
…charts into feat/box-plot-labels
|
There are five labels (numbers) for each boxplot item, three of which is on the left side and two right. But some developers may have different requirement like only displaying the middle value or display all values on the left. How can that be done? |
|
@Ovilia I would vote to not add an option to display for example all labels on one side because they most likely will overlap and I do not see a use case for that (if there is someone with that specific use case - we can always open up a follow-up) - I can however add an option to specify which labels should be displayed |
|
@Ovilia the user now has the option to set |
|
Hey @Ovilia are there any updates on this - do you need further changes from my side? Thank you very much :) |
src/chart/boxplot/BoxplotView.ts
Outdated
| el.z2 = 100; | ||
|
|
||
| const itemModel = data.getItemModel<BoxplotDataItemOption>(dataIndex); | ||
| const emphasisModel = itemModel.getModel('emphasis'); | ||
|
|
||
| if (seriesModel.option.label.show) { |
There was a problem hiding this comment.
We don't access the attributes using seriesModel.option. Please use const labelModel = seriesModel.get('label'); const show = labelModel.get('show'); instead.
Please also change seriesModel.option in other places.
src/chart/boxplot/BoxplotView.ts
Outdated
| if (seriesModel.option.label.show) { | ||
| const formattedLabels = | ||
| ((seriesModel as BoxplotSeriesModel).getRawValue(dataIndex) as number[]) | ||
| .splice(1).map((value: number) => seriesModel.option.label.formatter(value).toString()); |
There was a problem hiding this comment.
You should probably not create a Text element and do all the stuffs by yourself. Instead, setLabelStyle should be called. See how this is done with line series. But in boxplot's case, there are multiple labels so you need to figure out how this should be done, but generally speaking, setLabelStyle should be called most likely.
There was a problem hiding this comment.
I would not add it to setLabelStyle as it is super specific code which only applies to the bar chart, so it doesn't really make sense in my opinion to add it a general util method. However, I agree that it should live in the labelStyle util. So I would vote to move it there and refactor it so it does not create a Text element manually. What do you think?
There was a problem hiding this comment.
@Ovilia any news on this? Is the proposed solution good enough?
|
Hey, |
| }, | ||
| label: { | ||
| show: true, | ||
| formatter: function (param) { |
There was a problem hiding this comment.
If formatter is not provided here, there will be an error. You should not assert it to be defined.
| if (labelModel.show) { | ||
| const formattedLabels = | ||
| ((seriesModel as BoxplotSeriesModel).getRawValue(dataIndex) as number[]) | ||
| .splice(1).map((value: number) => labelModel.formatter(value).toString()); |
There was a problem hiding this comment.
This is not the correct way to format labels. If formatter is not defined, this should go wrong. You should probably do like sunburst.
There was a problem hiding this comment.
@Ovilia I am not sure if I understood correctly - getFormattedLabel is defined to return a string but somehow returns an object when being called with dataIndex.
I think I am missing something here, could you clarify how you would implement the formatting of the labels?
Thanks
| z2: 1000 | ||
| }); | ||
| const defaultOffset = 5; | ||
| const defaultXOffset = (ind % 2) === 0 ? (-(label.getBoundingRect().width + defaultOffset)) : defaultOffset; |
There was a problem hiding this comment.
As I said before, we should provide a way to let developers decide where to display the label to the left or right. The label.align can be changed into 'left' | 'right' | 'center' | { iqr: 'left' | 'right' | 'center' , ... }.
There was a problem hiding this comment.
@Ovilia Sorry, maybe I am missing something here as well but there is no single label. We have five labels, right? I still don't get how someone could choose the align for the five in a way that makes sense - e.g. all labels on one side - even though they might overlap?
|
@DanielBogenrieder Sorry for late reply. I think this PR still need modification before merging. |
|
Hey @Ovilia, Greetings and thanks, |
|
Hey @Ovilia, Greetings and thanks, |
|
Hey @Ovilia, Thanks, |
|
Any news on this? Anything we can do to get it merged? |
|
Is there an option to configure display labels in the current official version. At present, it seems that there is no mention in the official documents. How can I achieve displaying the median at the top? |
Brief Information
Box plot labels for all displayed values.
This pull request is in the type of:
What does this PR do?
This PR implements labels for all visible/plotted values of the box plot (this also includes the whiskers).
Details
The box plot now utilises the basics of the label option (i.e.
show,offsetandformatter) to display labels for the values of each box of the box plot.As the multiple labels are not supported for single series items - the
setBoxLabelsmethod was added to theBoxView. In order to custom calculate the positions and add the labels to the plot.Document Info
One of the following should be checked.
Example