fileserver: browse template SVG icons and UI tweaks

[glowinthedark]

Description

• continuation of fileserver: optimize browse template to avoid SVG icon duplication #5806 (my bad; github closed the pr automatically after force pushing a clean branch; lesson learnt 😄 )

1. Add folder symlink icon as current master displays folder symlinks as file symlinks
2. Make <path> tags self-closing in SVG.
3. Minor ui fix: in search field use <input type="search"> instead of text

---

@francislavoie: in reply to #5806 (comment) and #5806 (comment)
Keeping things simple is probably a higher priority than achieving some imaginary 'optimization' at the cost of adding extra complexity. I've updated the pr to only add the folder-symlink and type=search for the search field — it adds a tiny ⛌ button for resetting the search when it has text; not a big deal but comes for free from the browser. I suppose that extra filtering on the go side would require extra traversals so I feel the extra complexity is not worth the cost.

Also added a null-check for el.querySelector('.sizebar-bar') which is not present in grid mode.

Slight off-topic: the template currently does not maintain the list-vs-grid state on navigation clicks. Is this a known issue and is there some plan to fix it in the future?

My current quick fix which I don't really like is to hijack link clicks when grid mode is active with something like this:

document.onclick = onDocumentClickHandler;

function onDocumentClickHandler(e) {
  let link = e.target;

  if (window.location.search) {
    if (link.tagName !== "A") {
      link = findClosest(link, "A");
    }

    if (link && link.matches(".file a, .entry a, h1 a")) {
      e.preventDefault();
      e.stopImmediatePropagation();
      document.location.href = `${link.href}${window.location.search}`;
    }
  }
}

function findClosest(el, tagName, className) {
  if (el.tagName === tagName &&
    (!className || el.classList.contains(className))) {
    return el;
  } else if (el.parentElement) {
    return findClosest(el.parentElement, tagName, className);
  }
  return null;
}

An alternative approach could be attaching click event listeners to all links with css class=entry|file but I wanted to avoid that.

I suppose there can be a better and nicer way, but asking here in case you think this sort of quick fix might be appropriate and can be included in the pr. Or do you think the issue is big enough in order to deserve its own thread?

[glowinthedark]

a shorter version of the above js to keep state of list/grid mode might be (adds listeners to all eligible links):

document.querySelectorAll('.file a, .entry a, h1 a').forEach(link => {
  link.addEventListener('click', e => {
    if (window.location.search) {
        e.preventDefault();
        window.location.href = `${link.href}${window.location.search}`;
    }
  });
});

(I've only used document.onclick because in my custom version I needed to hook to that event anyway, it's a wip experiment to turn browse.html into a 'media server' )

[mholt]

Thanks for this change! Looks good.

Slight off-topic: the template currently does not maintain the list-vs-grid state on navigation clicks. Is this a known issue and is there some plan to fix it in the future?

Yeah, I was aware of that when I implemented it. I haven't planned to make the layout sticky but I guess we could do that if people prefer; I just wasn't sure whether that is the right thing to do. Just because this folder is shown in grid view, does that mean the next one we click into should be also?

(If we do it, we'll just need to do it without cookies.)

We could maybe do it the same way we preserve the sort settings... with a little server-side help through the query string.

[glowinthedark]

@mholt: persistent layout mode is probably for most people already muscle memory; and it's kind of unexpected when it is 'forgotten'. All file navigation UI's persist the mode, including mac finder, windows explorer, even linux file managers; web UI normally do the same; but maybe it's too small of a nuisance for people to even bother to complain about it? 😄

for sorting explicit get parameters are set on the sorting links; by following the same approach do you mean appending the grid mode params to all navigation links in the template or something else? isn't that going to be too verbose and increase the file size?

btw, maybe writing the current mode to localStorage would make more sense?

[mholt]

@glowinthedark That sounds good to me. The main problem is that the server needs to know about the layout choice. So that's why I chose query string.

Do we want to merge this in, and then we can address that in another PR?

[glowinthedark]

@mholt: yes, the grid mode persistence is outside of the scope of this pr; I only mentioned it in order to understand better the context; there is nothing more to add here in terms of the template code

as for the offtopic regarding the grid mode I have no clue what a clean way might look like; an even uglier than js approach could be storing a flag in localStorage and then on page load if the flag is found (preferably before anything is rendered) check for the flag and redirect from /some/dir to /some/dir/?layout=grid which imo is even worse than link click interception.

[mholt]

Awesome, thank you for this PR!

Yeah, I'm not sure the best way to go about the grid persistence thing. Might need to experiment...

[glowinthedark]

@mholt Thank you Matt for taking time to look at it!

As for the grid more I'll keep thinking.. A wild idea would be to experiment with a PWA-style template that won't need a page reload at all and would render everything client-side exploiting the fact that file_server is capable of serving JSON when called with Content-Type: application/json which can be htmlized with moustache or raw js, might not be practical but could be fun to play with. 😄

[mholt]

Thanks. Preferably I'd like to keep this a simple server-rendered app if possible... but yes we could potentially do crazy things. Maybe we could persist the layout in the query string, we'd just have to rewrite the link hrefs.