Imviz: Move coordinates overlay to toolbar

[astrofrog]

This moves the coordinates and data value info to the main toolbar:

This is to try and make it look like the proposed UI design - note that I had to widen the toolbar to accommodate this.

I also had to add a dictionary containing loaded tools on the application handler, which I'm not super happy about, but not sure if there is a more elegant solution (this is to allow viewers to have access to the tool instances)

EDIT: Close #545 , fix #514

[astrofrog]

@mariobuikhuizen - what would be the cleanest way to make this be conditional on whether a certain tool is in the bar? I only need it to not be dense if we are using g-coords-info

[astrofrog]

Actually i think we want to do this based on whether we are using imviz or other apps - I have an idea for how to do that

[rosteen]

If you look in the gaussian_smooth plugin vue file I'm pretty sure there's an example of checking which config the app is in.

[astrofrog]

Ah yes I see - but in this case what I need is the actual instance of the tool which I think we don't keep track of otherwise.

[astrofrog]

This is ready for review

[pllim]

I am not sure if my approval means anything here. This should be reviewed by the people who bless UI/UX features.

[ibusko]

I'm not an expert in glue/vue wizardry, but I approve since the code looks good and the app behaves as expected.

[pllim]

• Text got cut off using Chrome on Linux.
• Lack of indentation for second line of World:.
• y= position still jumps around when x= number of digits changes.

[Jenneh]

This looks good! Some polish:

• Adjust line height so that there is slightly less space between the 3 lines of text (my hope is this help to condense the 3 text lines together and make room for some more margin at the top and bottom)
• Can the font be a little smaller? (will it still be readable?) If possible, it will help to add more space around the text
• Add more margin (even an extra 5 px would help) between the text and top and bottom edge of the toolbar
• Remove the colon ":" after the title. The bold text does enough to differentiate it as a title

[rosteen]

My only thought is that to condense it a bit, you could put the two shorter outputs (pixel and value) on the same row, with world coordinates beneath them. So two rows rather than three, which might avoid needing to make the whole toolbar taller.

[Jenneh]

That might help!

[astrofrog]

I've now updated this as follows:

• Changed to a two-line layout as suggested by @rosteen
• Removed colons as suggested by @Jenneh
• Padded pixel coordinates with zeros to avoid too much jumping around to address @pllim's comment (there is no other way to easily prevent this unless we switch to e.g. a fixed-width font which might look ugly)
• Prevented line wrapping on narrow displays also to address @pllim's comment. On very narrow screens the information my then end up being cut off but I'm not sure how we want to address this. This does make me wonder whether we want to set ourselves a minimum app size below which we aren't going to address issues?

Preview:

[astrofrog]

And now with units when present:

[pllim]

being cut off

I think cutting off provides a stronger hint that I need to resize the browser. Wrapping just confused me. So I am okay with this if you cannot set a min size. Setting min size can also be a problem because what if someone opens this up on a tiny phone (when/if this is deployed as web app or something)?

[pllim]

@astrofrog , it is not padding enough when image is larger (e.g., 2k x 2k or 4k by 4k); i.e., "y" and "Value" still jump around. Can the padding not be dynamically set based on max dim of current active display?

No more wrapping though!

[pllim]

Speaking of jumping around, since we're adamant to display unit of the value, it is also distracting to have the unit jumping around, but I am not sure if there is a simple way to pad the values (maybe use scientific notation?).

[astrofrog]

@pllim - I've rebased this following the merging of your parsing PR, and I think there should no longer be any jumping issues with the latest commits?

[pllim]

Some final nitpicks, though we can defer to future PRs if you don't think they are blockers.

1. Looks like x and y are zero-indexed. People used to DS9 would expect 1-indexed. Perhaps just a matter of documentation.

2. When data does not have WCS, does it make sense to show "World"?

3. When cursor is out of bounds, does it make sense to show anything?

4. Does it make sense to show fractional pixel? For example, this one is when the cursor is at the edge of x=4 (y is centered at 18 already; both 0-indexed). I don't think we have sub-pixel resolution (and it is not something we said we would support). It is confusing to see x=3.6 when I know it is x=4.

5. The unit of "Value" still jumps a bit when value switches between + and - but I think we can ignore this unless user complains down the line.

[Jenneh]

I think when the cursor is out of bounds we should show nothing. My gut is that the text appearing will draw attention in a good way. We will have to get some feedback from users to be sure.

[astrofrog]

Looks like x and y are zero-indexed. People used to DS9 would expect 1-indexed. Perhaps just a matter of documentation.

I'll defer that to POs to decide how to handle this and can probably be done in a follow-up PR?

When cursor is out of bounds, does it make sense to show anything?

No, fixed

Does it make sense to show fractional pixel? For example, this one is when the cursor is at the edge of x=4 (y is centered at 18 already; both 0-indexed). I don't think we have sub-pixel resolution (and it is not something we said we would support). It is confusing to see x=3.6 when I know it is x=4.

Other tools usually show fractional pictures - probably a question for the POs?

The unit of "Value" still jumps a bit when value switches between + and - but I think we can ignore this unless user complains down the line.

Yes I'm not sure how to fix that beyond switching to a monospace font (:vomiting_face:)

I think this should be good to go otherwise!

[pllim]

I am okay with remaining items as follow-up issues. OSX failure is unrelated and would be solved by #580 .

[rosteen]

@astrofrog can you pull down the most recent main branch and rebase? We merged a bunch of CI-related PRs since this was created.

[astrofrog]

@rosteen - done!

[pllim]

Follow up issues at #590 and #591. Unless told otherwise, I will assume they are not in scope for MVP.