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

Providing more control over hover label in plots #1046

Closed
wants to merge 12 commits into from

Conversation

AlanRace
Copy link
Contributor

@AlanRace AlanRace commented Jan 6, 2022

I found that the label displayed at the cursor when hovering on a plot is only shown if either show_x or show_y were set to true. I wanted to be able to display the label regardless of whether the line guides were shown or not, and so added in the show_hover_label function to control this display.

I also found that show_x and show_y weren't very descriptive names for what functionality they were controlling (originally I thought it was related to the x-axis), so I have suggested a change in name to show_hover_line_x, but feel free to suggest something more fitting. Similarly with custom_label_func.

Finally, I modified the definition of CustomLabelFunc to enable a custom function to essentially recreate the default (this wasn't previously possible as it didn't have access to show_x and show_y.

An example of a custom label
Custom label

Copy link
Contributor

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Very nice! I also like the internal simplification (better naming + integrating to PlotConfig).
I have a few additional ideas to make things more intuitive/consistent 🙂

Comment on lines 232 to 233
let x_decimals = 6 - ((value.x.abs().log10()).ceil().at_least(0.0) as usize).at_most(6);
let y_decimals = 6 - ((value.y.abs().log10()).ceil().at_least(0.0) as usize).at_most(6);
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be used in a few places, also in bar.rs and box_elem.rs -- maybe extract it to a function?
A few similar helper functions are already at the end of items/mod.rs.

Comment on lines 23 to 24
type CustomLabelFunc = dyn Fn(&HoverConfig, &str, &Value) -> String;
type CustomLabelFuncRef = Box<CustomLabelFunc>;
Copy link
Contributor

@Bromeon Bromeon Jan 17, 2022

Choose a reason for hiding this comment

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

Since CustomLabelFuncRef seems to be the prevalent type used everywhere, maybe we should think about a shorter/more precise name? BarChart and BoxPlot have an element_formatter() method, which customizes the displayed text when hovering over them, which is essentially the same concept.

Maybe something like HoverFormatter?

On the other hand, the type dyn Fn(&HoverConfig, &str, &Value) -> String seems to be used only once, so it can either be inlined or use a more verbose name HoverFormatterFn or so.

We are going to need a lot of these customization functions, so agreeing on a term like "formatter" could make sense.
Use cases:

  • Hovering for bar charts and box plots (currently element_formatter, could be renamed to hover_formatter)
  • Hovering for X/Y values (this PR, maybe hover_formatter makes clear it's the same idea)
  • Labeling X/Y axis (#1130, with x_axis_formatter + y_axis_formatter)

@emilk
Copy link
Owner

emilk commented Jan 17, 2022

It would be great if a demo of the new feature was added to egui_demo_lib/src/apps/demo/plot_demo.rs, and perhaps a screenshot added to the PR description

@AlanRace
Copy link
Contributor Author

AlanRace commented Feb 3, 2022

I have now updated with the comments above and added a new demo. A screenshot of the demo is also included in the original PR description.

Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Looks great, but I think HoverLine makes more sense as a struct than an enum

X,
Y,
XY,
}
Copy link
Owner

Choose a reason for hiding this comment

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

Seems like it would make more sense to make this into a

pub struct HoverLine {
    /// Highlight values on the X axis by painting a vertical line.
    pub x: bool,
    /// Highlight values on the Y axis by painting a horizontal line.
    pub y: bool
}

@Titaniumtown
Copy link
Contributor

This PR looks really cool! Would love to see this feature merged.

@emilk emilk marked this pull request as draft April 3, 2022 18:26
@emilk
Copy link
Owner

emilk commented Apr 3, 2022

Coverted to a draft until work resumes!

@AlanRace
Copy link
Contributor Author

Is this still necessary after merging #1235?

@alexespencer
Copy link

alexespencer commented Jun 10, 2024

I think it is still needed even though #1235 has merged as this can disable the hover tooltip completely (as well as allowing show_x or show_y to be false but still having a tooltip)?

@emilk
Copy link
Owner

emilk commented Jul 15, 2024

egui_plot has recently been moved to its own repository, at https://github.com/emilk/egui_plot

This will hopefully speed up its development by having more reviewers and maintainers.

Please re-open this PR at https://github.com/emilk/egui_plot/pulls

See also:

@emilk emilk closed this Jul 15, 2024
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.

5 participants