2807: Labels now have a 0.5 outline #2957
Conversation
|
@bnelson05 this issue is actually about adding some sort of outline to all of the main places where users see labels: /audit, /validate, and /labelMap! I think that the one you've done already was most likely the hardest one though! 😁 |
|
Here is the thinner border style (about 0.5px - 1px). Here is the thicker border style (about 1.5px). |
|
I think I like the thinner outlines better almost always. I forget, have we tried swapping black and white as well? |
|
Here's what that looks like with the thinner outlines. |
|
Could you actually show an example for each of the four main label types (curb ramp, missing ramp, obstacle, surface problem) with black on the outside and white on the outside? |
|
Black Outlines: White Outlines: |
|
Okay I think I'm preferring the white on the outside! Thanks @bnelson05 ! |
|
I also prefer white on the outside! :) Thanks @bnelson05. Great work! 🚀 |
|
Final Outlines:
|
|
This looks really good so far!! Just a couple small visual updates to make. First, in Gallery on the expanded view, the border is making the label a bit off center. But I realized that if you increase the width/height of the marker from 20 px to 22 px it looks okay (because the underlying image is 20 px wide). Here are pics before and after I made that change manually. The other issue is that on Validate we have a "hide label" feature that mostly hides a label so that users can see underneath it. The border is being kept when we hide the label, and it should probably be removed in that case. Here's how it looks on develop vs this branch rn. |
bnelson05
force-pushed
the
2807-outlines-for-new-label-icons
branch
from
2dbbb1f
to
5fa9859
Compare
|
I have updated the width/height of the marker from 20px to 22px, and I added a CSS class called |
There was a problem hiding this comment.
A few more small visual things :) Pardon the nit-picking, we want this to look perfect, and it's so close!
I'm getting that same issue on /validate that I was getting on /gallery before, where the border is making the icon a bit off center. This was probably always happening on /validate, I just likely didn't happen to test with the obstacle label type where this is most obvious.
And on /validate, this a bug was introduced in your code. If you hide a label and then validate it while it is hidden, the button is continues to say "show label" even though the new label is shown by default. Then if you click the button, the text switches back to "hide label" and the icon doesn't change. Then if you click the 'hide' button again, it hides.
The gist is that you now have to click the hide/show button twice if you validate a hidden label. Here it is in pictures!
Hide first label
Validate it and a new label appears (unhidden). But the button says "show label" instead of "hide label"
After clicking "show label" it switches to saying "hide label"
Now when you click "hide label" it is hidden
| @@ -32,6 +32,8 @@ function LabelVisibilityControl () { | |||
| function unhideLabel () { | |||
| let panomarker = svv.panorama.getPanomarker(); | |||
| let label = svv.panorama.getCurrentLabel(); | |||
| var marker = document.getElementById('validate-pano-marker'); | |||
| marker.classList.add('icon-outline'); | |||
There was a problem hiding this comment.
I think that you can probably replace these two lines with just
panomarker.marker_.classList.add('icon-outline');
We're already getting the panomarker here, so it's just about getting the actual HTML element out of it.
There was a problem hiding this comment.
If you do end up needing to save a variable, try to use let instead of var if possible!
There was a problem hiding this comment.
I tried to combine the two lines in
LabelVisibilityControl.jsas you suggested but it didn't work the way I thought it would, so I just changedvartolet
Interesting... I just tested it out locally while using your branch and it worked exactly the same! In fact, if I log both of these to the console, I get identical output:
console.log(document.getElementById('validate-pano-marker'));
console.log(panomarker.marker_);
|
I centered the obstacle label in Validate and fixed the issue with the hide/show button. I tried to combine the two lines in |
There was a problem hiding this comment.
We are so close, but there is one new bug!
Whenever you hide a label and then validate it while it's hidden, you will see this error in the console.
This normally doesn't cause an issue, but if you do it on the last label in a mission, you are never able to start the next mission! The 'Validate more labels' button is remains grayed out and inactive.
…m/ProjectSidewalk/SidewalkWebpage into 2807-outlines-for-new-label-icons
|
The Uncaught TypeError bug should now be fixed. |
There was a problem hiding this comment.
Thanks for this amazing PR @bnelson05!!
In the interest of time given that it's your last week, I made a tiny update to the comments and removed some whitespace you accidentally added. You can see that commit here.
And then I added the outline to labels that I just added to the user dashboard last week, since that's something that you didn't have access to while you were writing this code! You can find that here.
Resolves #2807
Using ctx, I drew a black and white outline around the label icon image.
Testing instructions