janeway icon indicating copy to clipboard operation
janeway copied to clipboard

Can't set relative URLs on custom nav items

Open mauromsl opened this issue 1 year ago • 2 comments

Describe the bug When creating a nav item that points to a site path, the resulting URL is broken

Janeway version hourglass-v1.0

To Reproduce

  1. On your press settings, select OLH as your theme
  2. Create a custom nav item that points to /journals
  3. Visit the link on your navbar
  4. See resulting URL is https://journals

Expected behavior Existing behaviour should be preserved and relative URLs should work as expected

mauromsl avatar Feb 07 '24 08:02 mauromsl

Thanks for logging this issue, @mauromsl.

I just tested this on an older version of Janeway, 1.5.0, and I could reproduce it there. I have also compared the Git diff between 1.5.0 and 1.5.2 to double check, for my own sanity, that I have not introduced any changes to core Janeway on the way to building the Hourglass theme that would change the URL resolution of nav items.

These things tell me it is an older bug. I believe the culprit is links formed like this, as they have been since 2018:

https://github.com/BirkbeckCTP/janeway/blob/70e0ebd11ce05173d5b36639f77cd60849af32c4/src/themes/OLH/templates/press/nav.html#L19

These templates should not be introducing a hardcoded forward slash as that results in a double slash when combined with a leading slash on NavigationItem.link, signalling to the browser that what follows is a netloc, not a path.

These templates should also be using the newer NavigationItem.url method, not NavigationItem.link. The url method is more robust as it calls the helper functions in utils.logic.

How would you like to proceed? As it is an older issue, we won't be changing users' expectations with 1.5.2. I could try to clean up these old templates ASAP, but Caroline is keen to see the supporter data merged. What's your sense of the priorities here? @ajrbyers hoping to get your thoughts as well. Thank you both!

joemull avatar Feb 07 '24 13:02 joemull

Thanks for investigating this @joemull. The supporters issue is the priority here, perhaps either @mauromsl or I can take a look a this in the next few days if we get a chance. Otherwise we can push this into 1.6.

ajrbyers avatar Feb 07 '24 13:02 ajrbyers