Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate and Explore no longer automatically resizing #3626

Open
misaugstad opened this issue Aug 22, 2024 · 7 comments · May be fixed by #3649
Open

Validate and Explore no longer automatically resizing #3626

misaugstad opened this issue Aug 22, 2024 · 7 comments · May be fixed by #3649

Comments

@misaugstad
Copy link
Member

Brief description of problem/feature

I'm not sure how, but I think I must have broken the auto-zoom for Validate and Explore? I remember making some small change to it recently on the Validate page, specifically to accommodate our new Validate UI, but I thought it was working and I don't remember touching the Explore page...

Regardless, I've messed with a lot of stuff lately, so it's probably my fault! Will look into it very soon.

System info (OS, browser, city, and local/prod/test)

Prod, test, and local! Tested Chrome, it was always disabled on Firefox.

@jonfroehlich
Copy link
Member

jonfroehlich commented Aug 22, 2024 via email

@misaugstad
Copy link
Member Author

The CSS zoom that we used to make it so that the UI filled the page (sorry I forgot to include a screenshot). And actually things seem even worse than expected. It seems like the more I'm zoomed out, the further the Explore page drawing of labels on the canvas is from where the mouse thinks we're hovering... Here's an example in a series of pictures:

First, this is right before I place a label, the curb ramp icon is my cursor at this time. You can also see that the auto-zoom isn't working here, with all the whitespace at the bottom.
Screenshot from 2024-08-22 09-49-14

I place the label, and it jumps over a bit (my cursor is in the same place it was in the prev screenshot). And this is different from how it can sometimes jump, documented elsewhere. This is because of the zoom issue, as you'll see in pic 3.
Screenshot from 2024-08-22 09-49-43

If I hover my mouse over where I had clicked to place the label, it's treated as me hovering over the label. BUT if I actually hover over where the label is on the screen, the tool doesn't actually think that we're hovering over the label! You can only click on the label to open the context menu if your mouse is located where you initially placed it, and I can't figure out a way to delete the label...
Screenshot from 2024-08-22 09-50-02

I'm worried that something changed that is unrelated to what I did, and the CSS zoom could just not be working the same way anymore. Partially I'm thinking this because no one has mentioned the Explore page being messed up, and the last release was over a month ago. I also think that my Chrome browser was just updated yesterday, so I want to test on Safari/Edge. This is now top priority because the Explore page is barely working. 💀

@misaugstad
Copy link
Member Author

UPDATE: I think that it's only an issue with the most recent version of Chrome. I just tested on my Mac, and there was no problem in Chrome or Safari. Updated to the newest version of Chrome, now there's an issue. Chrome on Linux was what I initially tested on (newest version), and that's where I noticed the issue.

@misaugstad
Copy link
Member Author

And the release notes from the most recent Chrome release: https://developer.chrome.com/release-notes/128

Standardized CSS zoom property
Updates the existing implementation of the previously non-standard CSS zoom property to align with the new standard. This changes various JavaScript APIs to align with the spec, changes zoom to apply to iframe content documents, and changes it to apply to all inherited length properties (previously it only changed the inherited font-size).

I need to look into whether CSS zoom in Chrome is salvageable, or if this just means that CSS zoom is donezo for our purposes. :/

I'll start by doing a little bit of research on it, but if I don't find an answer quickly, then I'm just going to stop our website from using CSS zoom on Chrome and push that to prod ASAP so that others don't run into the issue while we look for a more permanent solution.

This was referenced Aug 22, 2024
@jsomeara
Copy link
Collaborator

jsomeara commented Aug 24, 2024

Could this, combined with negative margin potentially work? Or are there fundamental differences between zoom and scale?

Edit: Doesn't look like this'll work. With scale, there's tons of issues with mouse like you mentioned earlier (just tested).

@misaugstad
Copy link
Member Author

Yeah, a lot of the issues here come from the way that we're determining where someone clicks and how we're drawing on the screen. One idea has been to use PanoMarkers on the Explore page like we do basically everywhere else, which would hopefully enable us to have to flexibility to dynamically resize the GSV window (#303).

Adding labels is just the central piece of Project Sidewalk so we want to tread lightly when messing with it, especially since there are a dozen things downstream that we would need to verify still work after making such a change. Using CSS zoom was kind of a hacky workaround that allowed us to make essentially no changes to the inner workings of labeling 😅

@misaugstad
Copy link
Member Author

For anyone testing if a fix is working, here's a short list of things to check to make sure that everything is working properly. If this is all looking good, I should probably get in there and see if I can break it in any other way, but this is a good place to start!

  1. For everything you test when labeling:
    1. test with a label on at least 9 points on the GSV window (top-left, top-middle, top-right, middle-left, etc)
    2. Also test at a few different browser zoom levels, I'd start with testing at 75%, 100%, and 125%.
    3. Test while resizing the browser window a bit to a somewhat non-standard shape.
  2. Adding a label should not cause the label to jump to a different location (it happens very slightly most of the time, but it shouldn't be a large jump)
  3. The context menu that automatically opens when you click on a label should open and close correctly when clicking on a label, and it should show up in the correct location
  4. Tooltips that show up when you over over buttons on the UI and when hovering over parts of the context menu should show up in the correct place
  5. A few values saved to the database need to be checked, and these are incredibly important! You should add labels to specific places on a pano on the develop branch, and then test with the exact same location on your own branch. The values in the db should be the same in both versions (there will be very slight differences because you won't click on the exact same pixel, but it should be very close). The values you need to check are in the label_point table, columns pano_x, pano_y, canvas_x, canvas_y, heading, and pitch. You can start with canvas_x and canvas_y, because the others are at least partially derived from those. But if everything seems to be looking good, all those values should be checked.

@jsomeara jsomeara linked a pull request Sep 9, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants