Skip to content

Add a "show in legend" toggle for items to hide them from the legend #85

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

s-nie
Copy link
Contributor

@s-nie s-nie commented Mar 3, 2025

Document what "name" is for and add a show_in_legend toggle to hide items in the legend.

As discussed in #82 (comment).

  • I have followed the instructions in the PR template

@oscargus
Copy link
Contributor

oscargus commented Mar 29, 2025

Would it make sense to have an Option<String> for name and setting None if you do not want to have a legend entry? Or will that lead to the same id for all None items and therefore the same properties?

Or maybe put another way: what is the benefit of being able to toggle legend visibility separately, rather than just setting the name to an empty String? (Or more explicitly, None per above.) Not saying that there are no, just want to understand the difference.

Edit: I read up a bit in #82, but I still cannot really get it. Wasn't it possible to just pass an empty String earlier?

@s-nie
Copy link
Contributor Author

s-nie commented Mar 31, 2025

Would it make sense to have an Option<String> for name and setting None if you do not want to have a legend entry? Or will that lead to the same id for all None items and therefore the same properties?

That was pretty much the behavior before #82, and I preferred that as well. @Wumpf my understanding is that this limits visibility checking as the name is also used for that?

@Wumpf
Copy link
Collaborator

Wumpf commented Mar 31, 2025

Or will that lead to the same id for all None items and therefore the same properties?

that's exactly the issue, yes. You could still provide an id explicitly, but accepting None would make this very non-intuitive.
Guess we could still hide empty strings in the legend (does it do that?) but then you have to add an id explicitly.

@@ -84,6 +88,13 @@ macro_rules! builder_methods_for_base {
self.base_mut().id = id.into();
self
}

/// Whether to show the item in the legend. Default: `true`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should check if empty names show up and update doc accordingly

Comment on lines +41 to +42
/// The given name will show up in the legend and serves to track the visibility of items.
/// Multiple items may share the same name, in which case they also share a legend entry.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well that's not entirely true though, it's the id that tracks all this. It's just that by default the id is created from that name.

Suggested change
/// The given name will show up in the legend and serves to track the visibility of items.
/// Multiple items may share the same name, in which case they also share a legend entry.
/// The given name is used to create the item's id and will show up in the legend.
///
/// Multiple items may share the same name (and id), in which case they also share a legend entry.
/// The item's id is used to for visibility tracking and to refer to items in the plot's response.

should probably expand doc on the id setter as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants