Skip to content

Conversation

@fatteneder
Copy link
Contributor

Description

Adds the ability to toggle plot elements by clicking on their label inside a Legend.
Mimics gnuplots toggling behavior.

demo_hideshow

The functionality relies on <:AbstractPlot objects having a visible attribute. If there isn't one, we still put a shade above the legend entry.

Type of change

Delete options that do not apply:

  • New feature (non-breaking change which adds functionality)

Checklist

  • Added an entry in NEWS.md (for new features and breaking changes)
  • Added or changed relevant sections in the documentation
    I think this not needed.
  • Added reference image tests for new plotting functions, recipes, visual options, etc.
    Should we add a ref image that uses pick and generates a plot with missing elements? And can I even add one myself?

@fatteneder
Copy link
Contributor Author

We should also make it so that either clicking the title or right clicking anywhere in the legend box toogles all entries.

@jkrumbiegel
Copy link
Member

That's cool! Also it's probably a behavior many people expect from a legend nowadays.

I'm wondering how this can be made to work with multiple axes that share a legend. Usually the legend items are constructed separately in that case, or one plot object is passed to the constructor as a proxy for all others. Which would mean that no or only one facet would have its plot hidden with the current technique.

@fatteneder
Copy link
Contributor Author

I think I have not used Legend enough to understand what you are describing.
Could you maybe provide a MWE of the setup you have in mind?
Or is the example with merge & unique in the docs here close to what you want to do? https://docs.makie.org/v0.17.13/examples/blocks/legend/index.html

@jkrumbiegel
Copy link
Member

I just mean that semantically, one legend entry often describes multiple plots across facets. So you'd expect all of them to turn off, but the legend might have been created referencing only one (or even none) of them. So this method needs to be able to accomodate that scenario, where a list of related plots is passed by the user somehow.

@fatteneder
Copy link
Contributor Author

fatteneder commented Sep 15, 2022

Still not sure if I understand correctly.
Are you maybe referring to this comment:

We should also make it so that either clicking the title or right clicking anywhere in the legend box toogles all entries.

Now I can see what the problem would be here.

Nevertheless I could gather some more things from your comment that we should take care of:

  • If there is no plot connected to a label, then we should disable the toggle. This can be done easily, I think.
  • If one entry in a legend refers to multiple plot elements (which can be spread across more than one Axis), we want that clicking the label hides/shows all of them simultaneously. I did not test yet, but I thought the current implementation might do that already, because the mouse event goes through all the <:LegendEntrys that belong to a label's element connection, cf. this https://github.com/MakieOrg/Makie.jl/pull/2276/files#diff-93b8d91868f7576e0971bdc75440cc6f9bfa0562952fbd630d52cd2a84ccce36R243-R247
  • Some doc examples show the use of LineElement etc. As it is now you can only add a single handle to that using the plot kwarg. We should allow to pass a vector of handles so that we can connect multiple plot elements to one legend entry, which (I think) is automatically done if you only use Legend.

Again, I am really sorry that I don't understand. Usually I am not that slow 😄

@jkrumbiegel
Copy link
Member

Yeah maybe your last bullet point covers what I mean. I was thinking of facet plots where you have the same line plot let's say 12 times across 12 axes, but you haven't referenced all of them at once when creating the legend. I didn't check your implementation yet but if you added a plots kwarg where one could pass a vector that could take care of it.

@fatteneder
Copy link
Contributor Author

fatteneder commented Sep 15, 2022

I added three more things.
I illustrate them with the following MWE:

using GLMakie, Random
GLMakie.activate!()

Random.seed!(145)

f = Figure()

ax  = Axis(f[1,1])
lb = lines!(ax, 0:4, 0:4, color=:blue, label="lines 1")
lo = lines!(ax, 0:4, -4:0, color=:orange, label="lines 2")
sb = scatter!(ax, rand(3), rand(3), color=:blue, label="scatter 1")
so = scatter!(ax, rand(3), rand(3), color=:orange, label="scatter 2")
x = LinRange(0, 4, 100)
slb = band!(ax, x, cos.(x) .- 1, cos.(x) .- 2, color=:blue, label="band 1")
slo = band!(ax, x, sin.(x) .- 1, sin.(x) .- 2, color=:orange, label="band 2")

ax2 = Axis(f[2,1])
x, y, yerr = 1:2:20, 5 * rand(10), 0.4 * abs.(randn(10))
bb = barplot!(ax2, x, y; strokewidth = 1, color = :blue, label="barplot 1")
bo = barplot!(ax2, x, -y; strokewidth = 1, color = :orange, label="barplot 2")

Legend(f[1,2], ax)
Legend(f[2,2],
       [ PolyElement(plots = [lb, sb, slb, bb], color = :blue),
         PolyElement(plots = [lo, so, slo, bo], color = :orange) ],
       [ "blue", "orange" ], "Colors" )

f
  1. Construct <:LegenEntrys from a vector of handles. Clicking the entry then toggles all the plot elements in this group. If you watch closely you can spot an issue where plot elements are toggled but their shades in other legends are not. I think this is what Julius mentioned above.
    demo_custom_legend

  2. A right click anywhere in a legend toggles all the entries in that legend.
    demo_toggle_all

  3. Middle click anywhere in a legend shows all elements again and following middle clicks just switch to toggeling.
    demo_sync

Side note: scatterlines does not provide a visible attribute. My bad, it does work.

We now put a hatched shade on top of a legend entry if at least one of
its associated plot elements is hidden. If all are hidden we put
a full shade.
Also removed middle mouse click and made the right click such that
it reset the visibility for all plot elements.
@fatteneder
Copy link
Contributor Author

fatteneder commented Sep 18, 2022

I reworked the toggling strategy again so that it addresses the problem of partly hidden elements. We now use a hatched box if not all plots of an element are either visible or hidden. If all are hidden then we put a full box. I also simplified the mouse interactions: Left click now toggles all elements of an entry and right click anywhere in the legend resets all all plots of all elements, no more middle click.

demo_new_toggeling

There is also a question about what to do with plot elements that are already hidden when the legend is created. Consider the following

using GLMakie, Random
GLMakie.activate!()

Random.seed!(145)

f = Figure()

ax  = Axis(f[1,1])
lb = lines!(ax, 0:4, 0:4, color=:blue,    label="lines 1", visible=true)
lo = lines!(ax, 0:4, -4:0, color=:orange, label="lines 2", visible=false)
Legend(f[1,2], ax)

f

The plot would start with only lb shown in the axis but with two legend entries which both appear to be shown. Clicking the second entry would show lo but put a shade on its legend entry. I think we have to live with that, otherwise it will be difficult to decide when to put a shade when there are multiple plot elements associated to a legend entry. The above example also appears artificial to me, because when I plot stuff I usually want to see it too (unless I tinker around with Block elements or so).

Otherwise, I think this would be ready for a review.

@fatteneder
Copy link
Contributor Author

fatteneder commented Sep 18, 2022

Here is a list of plot elements for which toggling does not work:

I could not test for the following ones because they cannot be to added to Legends yet. IIUC the problem here is that there is no default implementation for legendelements, cf. #1703.

  • heatmap
  • image
  • annotations and text (might not even make sense)
  • mesh
  • meshscatter
  • volumeslices
  • volume
  • surface
  • wireframe

Last but not least: The legend symbol for arrows could be improved. Right now it works like that of a scatterlines, but we could make it so that the arrow head sits at one of the ends of the symbol (or at both).

@fatteneder
Copy link
Contributor Author

The WGLMakie failure seems to be due to a problem with LinePattern.
However, I cannot test locally because test WGLMakie fails for a different reason on my machine.

@fatteneder
Copy link
Contributor Author

This here might explain why the WGLMakie tests are failing:

"pattern barplot", # not implemented yet

@fatteneder fatteneder mentioned this pull request Oct 20, 2022
2 tasks
@ffreyer
Copy link
Collaborator

ffreyer commented Apr 9, 2025

I merged master and

  • refactored event handling to avoid creating multiple mouse state machines (this is rather slow, especially with WGLMakie I believe)
  • fixed and simplified right click (dropped checks to avoid visible triggers - these should be pretty cheap)
  • did bit of general cleanup (rename some variables, avoid some temporary arrays, clean up visible listeners, fix cleanup of entryrects)

TODO:

  • Right click doesn't "toggle" but makes everything visible.
  • Middle mouse button interaction is gone
  • Safeguards against legend entries that do not keep track of plots (deprecate old struct and warn?)
  • fix test errors
  • add new tests/refimages
  • add example in docs

If you watch closely you can spot an issue where plot elements are toggled but their shades in other legends are not.

Need to think about whether this is fixable without too much effort

@ffreyer
Copy link
Collaborator

ffreyer commented Apr 10, 2025

If you watch closely you can spot an issue where plot elements are toggled but their shades in other legends are not.

Need to think about whether this is fixable without too much effort

Ah this is not the case anymore,

I think I added/updated everything that's necessary to merge this pr now. Refimg tests looked a expected in e87e2e3. Benchmark plots looked fine too

@github-project-automation github-project-automation bot moved this to Work in progress in PR review Apr 10, 2025
@ffreyer ffreyer moved this from Work in progress to Ready to review in PR review Apr 10, 2025
@warn "LegendElements should now keep track of the plots they respresent in a `plots` field. " *
"This can be `nothing` or a `Vector{Plot}`. Without this, the Legend won't be able to " *
"toggle visibility of the associated plots. The `plots` field is missing in: $bad_types"
end
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we just use something like this?

function get_plots(le::LegendElement) 
    hasfield(typeof(element), :plots) && return le.plots 
     @warn "..."
    return nothing 
end

for element in entry.elements
if (element !== nothing) && hasfield(typeof(element), :plots) && (element.plots !== nothing)
for plot in element.plots
!hasproperty(plot, :visible) && continue
Copy link
Member

Choose a reason for hiding this comment

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

I feel like we should just guarantee that every plot has a visibility field.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should, but I see that as wasted effort before #4630.

function get_plot_visibilities(entry::LegendEntry)
visibilities = Observable{Bool}[]
for element in entry.elements
if (element !== nothing) && hasfield(typeof(element), :plots) && (element.plots !== nothing)
Copy link
Member

Choose a reason for hiding this comment

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

should use the get_plots(le)

@SimonDanisch
Copy link
Member

Closing in favor of: #4927

@github-project-automation github-project-automation bot moved this from Ready to review to Closed in PR review Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

9 participants