ualbertalib / can-link

Front end react app for CanLink project
1 stars 0 forks source link

Have the "Open Full Record" link open in a new tab #51

Closed sfarnel closed 4 years ago

sfarnel commented 4 years ago

When you click on an item in the result list, and then on "Open Full Record", the record opens in the same tab. Unfortunately, then when the user hits the browser back button, their most recent search is gone and they go back to the original. For example, if I search music from the home page I get the following (note that results are page 1-140):

image

If I then, say, limit years to 2018-2019 and click on Search I get the following (note fewer results):

image

If I then click on the first item in the result list and then on Open Full Record I see the following (in the same tab):

image

The only way to get back to where I was from here is to use the browser back button. But when I do I don't get back to my most recent, scoped search, but rather to the original one:

image

Wondering if the easiest and quickest fox for this is to have the full record open in a new tab?

jchartrand commented 4 years ago

A problem we'll have here is one that I think we talked about before:

The nginx web server isn't recognizing links for anything other than the root (http://206.167.181.124 or http://206.167.181.124/index.html).

We need to tell nginx to redirect requests for the record and search pages to the index.html

So links like:

http://206.167.181.124/record/787409f30cfd33b5d34eef37bbc08fe5

and

http://206.167.181.124/search

jc

sfarnel commented 4 years ago

Thanks @jchartrand @danydvd is this something you can look into?

danydvd commented 4 years ago

@sfarnel I can allow the non root requests. However, I am not entirely certain that it is safe. I will investigate the security repercussions.

sfarnel commented 4 years ago

Super; thanks @danydvd

jchartrand commented 4 years ago

@danydvd

Just to give more information, react is a SPA (single page app) and loads up the entire site (all pages) on first load (all the pages are in the single javascript bundle). So, when moving from the landing page to the search page, or from the search page to the record page (or vice versa), the app doesn't load the new page from the server - since it already has it in memory - it just shows the appropriate page.

The problem is when someone tries to load either the search or record page directly without first going to the landing page. React expects you to configure the web server (nginx in this case) to always load index.html, regardless of which page is requested, but then tell index.html which page was requested, so react can decide which page to show.

So in terms of security, you aren't actually ever loading anything other than index.html.

The most common way to do this in nginx is described in posts like these two:

https://stackoverflow.com/questions/43951720/react-router-and-nginx https://www.barrydobson.com/post/react-router-nginx/

danydvd commented 4 years ago

Thanks @jchartrand I believe we talked about this before and we made changes to nginx.config. I just double checked and the location redirect command is already in sites-available/default (which is included in nginx.conf):

server {
        listen 80 default_server;
        listen [::]:80 default_server ipv6only=on;

        #root /usr/share/nginx/html;
        root /var/www/canlink.ca/html;
        index index.html index.html;

        # Make site accessible from http://localhost/
        server_name localhost;

        location / {
                # First attempt to serve request as file, then  
                # as directory, then fall back to displaying a 404.
                try_files $uri /index.html;
                ...

This does not work for our record pages (although in theory it should try and redirect to index.html).

I was looking at some more options and based on Nginx documentations I added a new location parameter to address /record/ request. This seems to be working now for records. However, we can't use this for getting back the search result page (if I add a /search/ parameter since there is no query parameter after search/ it would simply redirect to index.html (landing page)).

@jchartrand @sfarnel what do you think? does it make sense if http://206.167.181.124/search takes the user to lading page (not a 404 error!)?

sfarnel commented 4 years ago

Thanks @danydvd can you clarify what you mean by 'records'? that is, before the change if a person was on a full record page and clicked back in the browser they would go back to the very initial search, which is what seems to be the same now (unless I'm mistaken). The hope was that it would work if there were any additional parameters set (such as date, or institution, etc.) Maybe I'm missing something?

danydvd commented 4 years ago

@sfarnel the issue is we should tell Nginx what to do with url patterns. For example to get to a specific record (e.g. http://206.167.181.124/record/b82a3ecfa149f7d874a505fe7cbb7f99) I need to tell Nginx to redirect to index.html (as this is how react works) and then use whatever comes after "/record/" to get to that particular page.

However, for "/search/" we do not have that extra parameters (e.g. for record we had "b82a3ecfa149f7d874a505fe7cbb7f99"). So the server does not know where to direct the page (since it does not have these parameters in the url (i.e. date range, subjects, ...).

For this method to work the url should be something like this (just as an example):

http://206.167.181.124/search/?q=query?date=date-range?subject=xyz

sfarnel commented 4 years ago

Ah, ok; thanks @danydvd got it now. @jchartrand can the url be constructed as such?

jchartrand commented 4 years ago

I think there are a few issues here:

  1. We need the independent 'record' page regardless of anything else, e.g.,

http://206.167.181.124/record/184b7703216b629f90cc617e5e54a7da

This is so people can cite the record, and when the link is opened, that record page will open. So I think your latest fix is essential.

  1. When you click through from a search to the record page, and then click back, the search form values that had been in the form are gone. Opening the record page to a new tab solves this in that the tab with the search form remains open, so there is no longer any need to click 'back'. So there isn't really any issue with clicking back to the search form (and so no issue with the nginx handling of http://206.167.181.124/search (although I don't understand why nginx doesn't just redirect all requests for /search, regardless of what comes after 'search;, back to index.html - why are the query params needed?)

  2. I could also fix the form so that if you click away and then back the values remain. I'd have to store the form values in a global state (using something called a react context). But maybe the new tab solution is fine, and in some ways possibly even better since people might want to keep multiple record pages open.

sfarnel commented 4 years ago

Thanks @jchartrand

re: 1. I agree - @danydvd please incorporate this re: 2. and 3. - I agree for now let's stick with opening in new tab; we can pursue this in more depth in future if desired

danydvd commented 4 years ago

Thanks @jchartrand

@sfarnel the first issue is already incorporated (clicking on http://206.167.181.124/record/184b7703216b629f90cc617e5e54a7da should work)

I will also add the /search/ to Nginx config so that refreshing the page does not throw a 404 error.

sfarnel commented 4 years ago

Thanks @danydvd the record page works for sure

jchartrand commented 4 years ago

@sfarnel @danydvd Thanks both. I've updated the link handling so it opens the record page in a new tab.

sfarnel commented 4 years ago

Excellent; thanks @jchartrand

sfarnel commented 4 years ago

Done. Closing