search

OneZoom / OZtree

OneZoom Tree of Life Explorer
Other
90 stars 20 forks source link

URL filling with nulls #785

Open jrosindell opened 7 months ago

jrosindell commented 7 months ago

Screenshot 2024-04-09 at 00 26 16

davidebbo commented 7 months ago

@jrosindell I'm not seeing this behavior on Chrome, using the same tour at the same step. My URL looks like https://beta.onezoom.org/life/@biota=93302?otthome=%40_ozid%3D1&tour=%2Ftour%2Fdata.html%2Ffrogs%404#x783,y1066,w1.4031

Maybe there are other factors at play?

lentinj commented 7 months ago

@jrosindell @hyanwong did you figure out what needs to be done to replicate this? When does an extra "null/" get added? Could you copy-paste what the full URL looks like?

I'd guess it's something to do with https://github.com/OneZoom/OZtree/commit/56cbc372fae9bdaa3b79ae94c27bc059962df4bd, but I've not many ideas how off the top of my head.

lentinj commented 7 months ago

I could also believe it only happens if you start the tour in a particular way. Going to a URL like https://beta.onezoom.org/life/?tour=%2Ftour%2Fdata.html%2Ffrogs%401 would start the tour on page-load, and since the original URL doesn't have a pinpoint in, it'd be null.

It's a very neat explanation, but unfortunately the URL above seems to work fine.

davidebbo commented 7 months ago

Right, I had tried a number of things, but could not reproduce this issue.

Not sure to what extent it's related, but I did notice that if you add random tokens to the URL path, they get preserved and the site otherwise works fine. e.g. https://beta.onezoom.org/life/random/other/tokens

lentinj commented 7 months ago

Yeah, that's how we're repeating the null/s. Everything before the final pathname part with the @ is treated as the base URL and preserved. Generally this is fine since we only modify the pinpoint at the end of the pathname.

A bodge would be to not record a URL if the pinpoint we're trying to record is null, but this would potentially break the share buttons.

lentinj commented 6 months ago

I still can't reproduce this (@hyanwong says that rapid fire "Next"ing can do it, but not here). However, the pull request should at least solve the repeated "null/"s.

I'm still not sure why there would be a state without a pinpoint mind, but the problem is somewhere around here:

https://github.com/OneZoom/OZtree/blob/2ee69002f00a21c963bc346e203ee4ba140f95f1/OZprivate/rawJS/OZTreeModule/src/navigation/record.js#L34-L48

Note that (line 34) refuses to record a URL without a pinpoint, but when in a tour we ignore this state and craft our own based on the current page location. If this URL doesn't have a pinpoint, it'd get passed through regardless.

I don't think the solution is moving this check. We still want to record the current tourstop in the URL, so we can then refresh / share the current URL.