Pop-ups don't record state in URL on reload

[lentinj]

When a pop-up opens, we record pop=ol_12345 in the URL. However, if the page is reloaded, this information is lost. Any further reloads will not show the pop-up.

Similarly, we don't remove the pop=ol_12345 on pop-up close.

We should be recording the state change in views/treeviewer/layout.html, on pop-up open/close/tab-change.

This could happen by the UI functions calling back to record_url({ record_popup { . . . } }), instead of doing it when the pop-up event happens.

Arguably, this shouldn't be happening in the treeviewer at all, and the UI should be managing the querystring parameter itself since it's nothing to do with the treeviewer, but this is probably being excessively pedantic.

[jrosindell]

Hmmm, good point. I feel inclined to support the pedantic view of this. We have a tree viewer module that we imagine could be used in all kinds of different pages with all kinds of different UIs - the tree viewer module will need to save and read key aspects of its state from the URL, the tours module will need to do the same and the UI will need to do the same. So, we should probably do this the pedantic way to make life easier in future and to keep things in their proper places. The only thing we'd need to watch out for would be clash of URL parameters between the different layers - good documentation of the module would mitigate this so any UI or app using the tree viewer (and possibly the tours as well) will know what not to do in order to keep its own state distinct from that of the tree.

[lentinj]

Yeah, I do kind of agree. record_url() should be leaving all querystring parameters it doesn't understand, and then the UI would be free to add pop=ol_12345 as it needs. This is good separation of concerns.

But for the sake of being right we've now got 2 places that URL options are defined / documented, navigation/utils.js & treeviewer/layout.html - as an external person, I don't know/care about the distinction between treeviewer & UI, I just want the URL parameters. This is possibly part of a wider point brewing about where documentation like this (and HTML tour schemas) should live.

We've also got a fair bit of extra complexity in both places to modify the URL without treading on others' toes. But the treeviewer half of that is going to be required at some point, like you say.

the tours module will need to do the same and the UI will need to do the same

The tour state is considered part of the tree state for now, as the tours module is part of the treeviewer, unlike the UI. It's not very tightly coupled to the treeviewer internals though, splitting it off would definitely be possible.