Give clim popup a minimum width and make sure it stays on screen when layer controls are floating

[tlambert03]

Description

This PR makes sure that right clicking on the contrast limits slider shows the popup in the main viewer window even if the layer controls are undocked/floating. It does so by adding a bottom up find_ancestor_mainwindow(widget) utility function, and slightly updates the QtPopup.show_at method. Would be nice to include this with #866 in the next bugfix release.

Type of change

• Bug-fix (non-breaking change which fixes an issue)

How has this been tested?

• added a test that the find_ancestor_mainwindow function works.
• all tests pass with my change

Final checklist:

• My PR is the minimum possible work for the desired functionality
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works

[sofroniewn]

This looks great, I was able to verified the old undesirable performance on master and the fix on this branch. Ideally I'd like us to wait until our CI issues are fixed until merge just so we know everything passes, but then this is ready for 0.2.10

[jni]

Are we positive this is the behaviour we want? e.g. if I've undocked the controls and moved the sliders to another monitor, I think if I right click my preferred behaviour is that I get a popup on the same monitor as the controls... It should not be the width of the controls window but rather much wider. Thoughts?

[tlambert03]

I'm not positive, no... but currently, the undocked layer controls are rather small by default ... so we either need to do this, or do what you say where the popup extends well beyond the edges of the parent window.
I can certainly tie it to the controls instead of the main window, but what do you propose as the minimum width to make it? that part seemed a bit arbitrary. Also, if someone has the controls on the left side of the monitor or the right side of the monitor, it requires a lot more logic to prevent the contrast limits popup from going off the edge of the monitor, and then when they click and drag the controls to bring it back into the monitor, the clims will disappear... whereas chances are, the main window is always in full view, and the contrast lims will be (temporarily) right above the image you're adjusting.
neither seems perfect... this seemed the lesser of the two evils to me, but I'm happy to change it if you'd like

[jni]

I don't think this is super urgent, and I'd rather take the time to get it right. Yes, doing it correctly does sound like a mess, but otherwise, we're back to the old napari, moving back and forth between controls on the left of the canvas and the ones on the right. I actually find myself getting frustrated just thinking about right-clicking on one part of the screen and having the controls pop up somewhere else 😂

[tlambert03]

can you clarify exactly what you would want the behavior to be then? assuming the popup always appears at the top of the layer controls... what minimum width? and if the layer controls are at the edge of the monitor, you want the popup to keep that minimum width but have the edge of the popup hit the edge of the monitor and then extend off-centered off the edge of the layer controls?

[jni]

Yes, that's exactly what I would expect, kind of like any other right-click menu really...

[sofroniewn]

How about at the top of the layer controls width defined by some fraction of the entire screen width (say 2/3 or 3/4 assuming it's easy to grab) and then either centered on screen if controls are in the middle or left aligned if controls are very far on left, and right aligned if very far on right (again assuming you can find out where the control panels are). Agreed this logic is a little more complicated, but I think we can then get it looking nice for the majority of configurations

[tlambert03]

sounds good. will work on it

[tlambert03]

ok, here's how it currently works:

that what we're going for?

[jni]

🎉 🚀 😍

[sofroniewn]

I just tested this and works great for me. I'd say good to go after fixing the windows test and addressing Juan's comment on find_ancestor_mainwindow

[sofroniewn]

One comment @tlambert03 - that can wait for future PRs - but if we like the ideas in #879 we might want to make the slider popup be vertically aligned with the slider your clicking on instead of with the top of the viewer. This could mean less mouse movements if you're clicking on many of these, maybe also if the control panel is docked underneath the layers list

[jni]

I'm 👎 on the vertical sliders... It seems weird. I haven't encountered that in any other software, and when I imagine it I think I would find it jarring.

[sofroniewn]

I'm 👎 on the vertical sliders... It seems weird. I haven't encountered that in any other software, and when I imagine it I think I would find it jarring.

I think you might be misunderstanding @jni / maybe I was very unclear with my wording - I'm just talking about the vertical alignment of the slider (i.e. how high / low it is on the screen), the slider is still horizontal.

Does this make more sense now? Or am I misunderstanding you. I can put a screenshot in if needed

[jni]

Ah! Yes, I totally misunderstood. 🤦‍♂️ All good! 👍

[tlambert03]

One comment @tlambert03 - that can wait for future PRs - but if we like the ideas in #879 we might want to make the slider popup be vertically aligned with the slider your clicking on instead of with the top of the viewer.

If @AhmetCanSolak wants to play with that idea in #879, I would suggest starting with the QtPopup.show_above_mouse function. That function should probably be modified to a more general show_relative_to_position function that accepts a QPoint as an argument (rather than just always defaulting to QCursor.pos(), and making that point the clicked widget would accomplish this behavior.