Horizontal Barchart does not print labels

[joshka]

Description

Horizontal Barcharts were added recently (0.23.0), but labels were not implemented.
Barcharts have two label types: label and text_value (a formatted version of the value), only the latter is rendered.

To Reproduce

let data = vec![
    ("Jan", 10),
    ("Feb", 20),
    ("Mar", 30),
    ("Apr", 40),
    ("May", 50),
    ("Jun", 60),
];
BarChart::default()
    .direction(Direction::Horizontal)
    .data(&data)
    .render(area, buf);

10█████

20█████████████

30█████████████████████

40████████████████████████████

50████████████████████████████████████

60████████████████████████████████████████████

Expected behavior

Jan 10█████

Feb 20█████████████

Mar 30█████████████████████

Apr 40████████████████████████████

May 50████████████████████████████████████

Jun 60████████████████████████████████████████████

Screenshots

Environment

version 0.23-alpha (main branch as of today)

Additional context

[Valentin271]

I can work on that.

I'm wondering how should we handle something like this.

let data = vec![
    ("January", 10),
    ("February", 20),
    ("March", 30),
    ("April", 40),
    ("May", 50),
    ("June", 60),
];

Bars should always be aligned so we have to have a max label length somehow.

I'd suggest having an option to configure the max_label_len (feel free to propose a better naming). If not defined, we'd use a default value - either an arbitrary value like 3, the longest label length or maybe the average length.

[karthago1]

this is not a bug, It is a feature to use the space more efficiently 😃 and force you to print what you need on the bar instead

[Valentin271]

I'll wait to see what maintainers have to say.

I understand your logic but then there is a useless setter when using horizonal bars.

I'd say it's better to add the label as requested here, so users have the choice, and document the fact that this will use space and that could be avoided by using your technique.

[joshka]

I don't quite understand the rationale / implementation in #518 of the length constraint. Looking at this test example:

BarChart::default()
            .direction(Direction::Horizontal)
            .bar_gap(0)
            .data(&[("January", 10), ("February", 20), ("March", 5)])
            .label_max_len(1);

Which renders as:

J 10██    
F 20██████
M 5█      

1. Why wouldn't a user pass in single letter labels instead of the full label?
2. Why does it make sense to do the truncation of the labels in the widget instead of outside?

[Valentin271]

I just thought it would be useful.

Why wouldn't a user pass in single letter labels instead of the full label?

I was thinking of some API (whether it be web, system, or from crate) that may return labels of different length. In that case it would be annoying to truncate the labels.
But thinking about it again, maybe it's not such a common use case.

Why does it make sense to do the truncation of the labels in the widget instead of outside?

Both way are perfectly fine, just giving the possibility to the user.

Overall, if you think this is too much, let's just remove it and only merge the fix.

[joshka]

+1 Let's take this bit out for now like you said.

it would be annoying to truncate the labels

Its this bit I'm confused about - if you're getting the data from somewhere else, the likelyhood it's exactly in the shape of &[(&'a str, u64)] seems low, so you're probably massaging it already and can add a [..1] on your string during that mapping just as easily as a .label_max_len(1)

[Valentin271]

Fair enough. Kept only the fix.