Imviz: Show coordinates in degrees too #971
Conversation
Codecov Report
@@ Coverage Diff @@
## main #971 +/- ##
==========================================
+ Coverage 70.06% 70.59% +0.53%
==========================================
Files 71 73 +2
Lines 5121 5391 +270
==========================================
+ Hits 3588 3806 +218
- Misses 1533 1585 +52
Continue to review full report at Codecov.
|
pllim
force-pushed
the
show-me-the-degrees
branch
from
a77e7b7
to
d043cc4
Compare
pllim
requested review from
duytnguyendtn,
ibusko,
javerbukh,
kecnry,
ojustino and
rosteen
as code owners
| @@ -2,5 +2,6 @@ | |||
| <div v-if="pixel" style="white-space: nowrap;"> | |||
| <b v-if="pixel">Pixel </b>{{ pixel }} <b v-if="value">Value </b>{{ value }}<br> | |||
| <b v-if="world">World </b>{{ world }}<br> | |||
| <b v-if="world"> </b>{{ world_deg }}<br> | |||
There was a problem hiding this comment.
Maybe @kecnry knows a better way to do this... 😅
Kinda hard to get them align perfectly from image to image, but hopefully this is good enough without having to specify a special div for each sub-element.
There was a problem hiding this comment.
You should be able to use <v-col> here I think, to make everything line up without this workaround.
There was a problem hiding this comment.
I can think of many other ways to do this... but they all require nesting more elements or providing a fixed width for your empty <b> element. But I think unless we want to structure this further, this is probably fine for now.
There was a problem hiding this comment.
So should I try out the <v-col> stuff, or leave it as like it is 1999?
There was a problem hiding this comment.
I guess using columns would be a little more font-safe (for non-fixed-width fonts where nbsp might not line up well) and would be more future proof to maintain down the road
|
I was going to say that I dislike how tall the toolbar is getting to fit this and recommend a toggle to switch between the two types, but then I saw that the toolbar seems to be just as tall on main. So I guess it's fine! |
|
The mouseover coordinates layout shown above looks good. Official review later... |
|
Too many significant figures for the decimal degree coordinates. I think 10 decimal places will cover most needs (microarcsec). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| <b v-if="world">World </b>{{ world }}<br> | ||
| </div> | ||
| <div v-if="pixel" style="white-space: nowrap;"> | ||
| <table> |
There was a problem hiding this comment.
My vuejs-fu is too weak. I think this accomplishes what you asked for, but I don't know how robust it is across browser and monitor settings...
There was a problem hiding this comment.
Looks good to me. At small widths, it overlaps the plugin menu button, but that button isn't clickable while mousing over the image anyways. We have another ticket to eventually deal with minimum widths and overflowing across the app, so I don't think you need to worry about that too much right now.
* attempt to fix padding inconsistencies between browsers and jupyter notebook vs lab
Attempt to fix vertical padding on pixel coordinate div in imviz
|
Kyle has fixed the padding problem, so I am going to merge when CI passes. Thanks, everyone! |
Description
This pull request is to address the first point mentioned in #598 .
🐱
@Jenneh, would be nice to have your input on this. Thanks!
Todo
Screenshots
Without this patch (HST/ACS):
With this patch:
Checklist for package maintainer(s)
This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.
triviallabel.CHANGES.rst?