Feature/explorer routing

[jgonggrijp]

This is quite a large set of changes. Full details in the commit log, summary below:

• Adds routing to the explorer, which enables deep linking to (small groups of) particular panels through the browser address bar and scrolling on navigation with the browser's back/forward buttons. This involves several new units:
  • A dedicated radio channel for panel events.
  • A router (currently in the frontend/src/navigation directory).
  • A utility function that panels use to report their specific deep link.
  • A separate module that holds a single source of truth about route patterns, to support the previous two units.
  • A route parser that optionally uses the controller to reset the explorer to a deep-linked panel or group of panels.
• Splits the former vague catch-all "readit" aspect module into exploration, navigation and registration aspect modules. I'm aiming to exemplify the project architecture better with these changes; I'll also upload an image to clarify the import hierarchy more in a followup post.
• Untangles the explorer logic, up to pragmatic limits:
  • Lifts explorer view and controller into global instances.
  • Removes circular reference between explorer view and controller.
  • Refactors explorer controller to remove duplicate logic and use more consistent names.
  • Moves controller event bindings to the exploration aspect module.
  • Reduces the explorer view to just a view with a couple of special methods (push, reset, overlay), which doesn't know anything about the controller or about specific panel types.
  • Reduces the explorer controller to a bundle of more specific operations on the explorer view.
• Lifts CategoryColorsView into a global as well.
• Implements Polish tabs and navigation #323.
• Various bits of structural cleanup and reorganization in order to make the above changes manageable. This includes some changes related to Reorder the frontend modules #290.

For live testing, I suggest a couple of actions:

• Open a bunch of panels.
• Scroll horizontally, allowing the scroll bar to settle. This will update the URL in the browser address bar.
• Use the browser's back and forward buttons. The explorer will scroll to the corresponding panel if it moved outside of the viewport.
• Open a deep link to a panel in a new browser tab.

There are still some rough edges and open questions, but I decided to submit the branch now because @BeritJanssen was waiting for it:

• Maybe the explorer router should be in the explorer directory. I consider this part of Reorder the frontend modules #290.
• When resetting the explorer, the right panel(s) will appear, but rendering of the panels tends to be incomplete because some (most?) panels are not robust against incomplete data yet. I suggest deferring this to new issues/PRs.
• When scrolling on history navigation, there is a quirk on leftward scroll. Scrolling is currently set up to minimize motion, scrolling just enough to the left or right to reveal the target panel. The address bar, on the other hand, always reflects the rightmost panel that is fully visible. This means that if you navigate back to the left, for example to /explore/item/14 (an LdItemPanel), and the window is wide enough to accomodate at least one whole other panel right of it, for example the RelatedItemsPanel with route /explore/item/14/related, then the route of that panel to the right will appear in the address bar, instead of the route that you navigated back to. This also erases the browser history that came after /explore/item/14. It is easy to fix, but there are two ways to do this and the consequences for the users are very different:
  1. Adjust the scrolling logic a bit to move the target panel always to the right edge of the viewport (but still only if it has moved out of view). This is entirely consistent, at the expense of some increased horizontal motion.
  2. Adjust the routing logic to report the URL of the leftmost fully visible panel when scrolling to the left. This keeps motion minimal, but users may be surprised when manually scrolling to the left and then copying the deep link from the address bar, if they expect to get the link to a panel that was more on the right.
• While I think the route parser is a neat solution in principle, I'm very aware that its current implementation is totally opaque. At the very least, it will require documentation and a unittest (which I'll happily add in a followup PR), but I'm also open to discussion and other solutions. The problem it solves is that when a new route is matched (e.g. /explore/item/100), the action to take is conditional: either scroll to a panel that was already open, or reset the explorer with a new panel. As a consequence, the reset actions cannot be directly bound to the routes. If first deciding to scroll or to reset, as is currently the case in the exploration aspect module, this requires reconstructing the panels that go with the route name on reset, which is the task of the parser. It might be possible to reverse this, i.e., match a particular route first, then decide between scrolling and resetting.

Fixes #62, fixes #289. Partially implements #106 (needs review).

[BeritJanssen]

Looks like a neat restructuring of the code! The only thing I doubted about was pulling together instance of view and data in global, e.g. ExplorerView instance and source instance: adding extra files might mean a developer needs to click through more files to find definitions. However, I think I understand the reasoning and agree that in the long run this is probably cleaner.

[jgonggrijp]

Thanks! You make a valid point about having many modules to search through. While I believe in the general pattern, the sources and source-list-view modules are a bit superfluous. They can be removed when we implement #322, which is lightweight enough to not require global persistence.

[BeritJanssen]

I'm excited with how well the routes adapt to opening new panels. However, going "back in history" might feel buggy to a user:

• if I click "edit" on an edition, then go back, the route doesn't change.
• if I click all the way back such that the address is {root}/explore, the view will get stuck at a panel which is not the source list, see screenshot.

- pasting a deep link may lead to a similar experience: only one panel, and no way to go back to the source list.

Both of this may have to do with point 2 of your "open ends", but I think they might interfere with user experience to some extend. I also noticed clicking the link "explore" doesn't bring back the source list. So while I approve of the code, I wonder whether it's wise to merge just yet.

Point 3, the horizontal scrolling, is something I think I would need to discuss in person. I think I prefer option (I) (more horizontal motion but consistent with current behaviour), but can't picture how much / in which cases that motion would occur.

[jgonggrijp]

Thanks for the excellent feedback! My plan for today is to address the concerns as well as I can, at least the low hanging fruits:

• Self-report the {root}/explore route from the source list panel so that it can be navigated back to, like all other panels.
• Reset the explorer with the source list when clicking on the "Explore" tab.
  • Preferably only after deep-linking, i.e., scroll rather than reset when the source list is already the leftmost panel.
• Implement option (i) to solve the inconsistent backward scrolling issue. We can still switch to option (ii) later.
• Try my hand at a more legible alternative to the route parser.

On the assumption that you need to build on these changes next week, I will probably merge whatever I have by the end of the day, unless you have a release planned on the very short term.

Some rough edges that I'll likely postpone until after the merge:

• What to do when navigating back to a panel that was overlaid. The current situation, as you pointed out, is that the overlay blocks the target panel from view and also pushes a new state to the browser history, hiding the route in the address bar. A naieve solution could be to just remove any intervening overlays when scrolling to a particular panel, but if you press the "forward" button again after that, this will result in a reset instead of a scroll because the target panel was destroyed. A more intelligent, but also much more involved, solution would be to implement the Command pattern (other explanation) so that the explorer and its history gain full undo/redo capability.
• Panels that render incompletely after deep-linking.

[jgonggrijp]

@BeritJanssen I'm still tinkering with this, but I already fixed most all of the low-hanging fruits and I pushed my fixes just now. If it suits you, you can do another test to check that the behavior is more palatable.

[jgonggrijp]

Earlier in this PR, I mentioned a RouteParser class, which was a somewhat opaque finite state machine. That class is gone, so hard-to-understand code shouldn't be an issue anymore.

Two main issues remain after this merge, which I'll be creating new tickets for, to be linked below.

Specific heads-up for @JeltevanBoheemen: You need to run yarn fyarn when working with a descending commit, because I updated a dependency. The aspect modules and the explorer modules are cleaner now.