Shared-Reality-Lab / IMAGE-browser

IMAGE project browser extensions & client-side code
Other
2 stars 0 forks source link

Handle static image google map embeds #240

Closed hongyec closed 1 year ago

hongyec commented 2 years ago

This resolve issue#235 It resolve that when google map is an image, the "Get IMAGE map rendering" won't pop up, and it will evaluated as an image, but not map.

Screen Shot 2022-07-19 at 5 24 02 PM
jaydeepsingh25 commented 2 years ago

@hongyec , did you test this code? I don't think this will work, you are not calling the function processIMAGEMAPImages anywhere.

jeffbl commented 2 years ago

@hongyec any motion on this based on @jaydeepsingh25 comment above?

hongyec commented 2 years ago

@jeffbl @jaydeepsingh25 sorry, forget to push, Tested and it works fine!

jeffbl commented 2 years ago

@hongyec Can you please give some detail about what cases from the original work item this resolves? It isn't obvious to me from looking at the diffs. After that, please ping @jaydeepsingh25 here to get this re-reviewed. Thanks!

hongyec commented 2 years ago

@jeffbl It resolve that when google map is an image, the "Get IMAGE map rendering" won't pop up, and it will evaluated as an image, but not map.

jeffbl commented 2 years ago

OK, got it. That is useful and I think the correct thing to do, but was not the idea behind the original work item, which this fix would not resolve. The goal of the original work item was to use the information in the associated link, or otherwise extract it, to actually get the lat/lng, which look like they should be available for the listed embeddings, so that we can render something useful in common cases, e.g., on sites like tripadvisor. Is that feasible?

hongyec commented 2 years ago

Hi @jeffbl, if I understand it correctly, we would like to get the lat/lng from the image maps on the example website, and in order to achieve that, we need to evaluate the link as a google map, right?

hongyec commented 2 years ago

@jeffbl I see what you mean, the link itself contains the lat/lng, and we would like to extract it. For example, in https://www.tripadvisor.com/Restaurant_Review-g34438-d23806008-Reviews-Mayami_Wynwood-Miami_Florida.html, we would like to extract g34438-d23806008 out of it, right?

jeffbl commented 2 years ago

Yes, exactly. I wouldn't do this if it was not general to multiple sites, or used consistently on a huge site like tripadvisor. So there may be some investigation for some of these to see what the most common cases are. But if not too time consuming to implement, even just getting it working with tripadvisor alone would be useful. Note the original item has several examples for other sites as well.

That said, this PR is still useful if it means we can send the actual graphic whereas we couldn't before, if it is a graphic of a map. But that would be assoicated with a separate work item you'd have to create, since this PR will not resolve #235. Happy to discuss if not clear!

hongyec commented 2 years ago

Hi @jeffbl, will you be available to discuss it tomorrow around 5:30pm or 6:00pm, I still feel a little bit lost on this work item?

jeffbl commented 2 years ago

This should not be linked to #235 since it does not resolve that work item. However, I cannot seem to unlink it here. @hongyec if you are able to do so, that would be great. I've created new work item #249 and linked it here since that is the issue this PR actually resolves. So, we can go ahead and review/merge this when ready, but it should NOT close #235, which still needs to be implemented.

jeffbl commented 2 years ago

@hongyec please assign to @jaydeepsingh25 when this is ready for review against #249

jeffbl commented 1 year ago

Assigning to @jaydeepsingh25 since have not heard from Hongye.

jaydeepsingh25 commented 1 year ago

Closing the PR as it has conflicts, will raise a new PR with proper fix.