Fix BoxPlot legend

[mkalte666]

Fvg on discord noticed that BoxPlots were missing the legend.

Upon further investigation, either some merge was a bit icky, or something was missed when adding in PlotItemBase.

To fix this i have

• Replaced BoxPlot::color with BoxPlot::default_color, as it was a name collision with PlotItem::color.
• Replaced BarChart::color with BarChart::default_color, as it was a name collision with PlotItem::color.

I know this is a breaking change without a deprication first, but i feel this is better than going for a deprication first keeping that name collision in for longer.

• Remove BoxPlots implementation of the PlotItem::name() function, and the BoxPlot::name field, as it was incomplete, and is also covered in PlotItemBase
• Reduced the visibility of BarChart::default_color and BoxPlot::default_color, as it is only read, and only in plot_ui.rs, and that can be done via our newly not-name-colliding PlotItem::color.

I also noticed there are missing snapshots for the (Charts.png is only the histogram, and only vertical). Barchart and boxplot are missing. But that is worth its own PR.

In any case, with this, i get the expected legend back:

• I have followed the instructions in the PR template
• I have run check.sh locally and it went through.

[frankvgompel]

Doesn't this fix imply that PlotItem::name() is completely redundant? All item names (that are used in the legend) seem already defined in the new() function.

[mkalte666]

Doesn't this fix imply that PlotItem::name() is completely redundant? All item names (that are used in the legend) seem already defined in the new() function.

PlotItem defines shared functionality between all the things that need plotting - such as Lines, BoxPlots, BarCharts, Points, and whatever else. Specifically in this case, the legend does not care what is providing a legend for - it just eats some form of dyn PlotItems in the end.

PlotItem::name() is a getter, not a setter - and indeed, the fields are set via new().

What i have done is remove the BoxPlot-specific implementation of PlotItem::name(), as the provided default implementation already gets the name from the PlotItemBase.
Additionally, i removed the BoxPlot::name field, as that was redundant with the PlotItemBase::name field, and is currently not correctly initialized in the new function anyway. (see here:

egui_plot/egui_plot/src/items/mod.rs

Line 1415 in 46b496c

name: String::new(),

).

Technically, just removing the name field and the name implementation is enough to fix the legend issue, i have however decided to - while at it - rename the BoxPlot::color function to BoxPlot::default_color, (and same for BarChart, as otherwise it would collide with the color-getter from PlotItem, and you would need to do some as PlotItem access to use it.

[lucasmerlin]

Nice! I've reverted the default_color change though, seems like the other plot items (e.g. Arrows, Points) also have this problem and if we fix this we should fix it for all. A seperate PR that fixes this for all items would be appreciated. I think a nicer name for the setter would maybe be with_color? And then keeping color() as the getter. On the other hand that would be inconsistent with the other setters, so maybe we should change them all to be with_*. Or change the getters on PlotItem to get_*? Seems like there is no nice solution 😅

Also note that you can still get the color via PlotItem::color(&my_item) (which is inconvenient but not super-bad)