Skip to content

cm blob view: Improve initial render, scroll into view and navigation between files

Warren Gifford requested to merge fkling/codemirror-blob-tweaks into main

Created by: fkling

This commit addresses minor issues with the initial implementation:

The useExtension hook as it were resulted in creating the editor without the configured extensions and only adding them via a transaction afterwards. While that should generally be fine, if something like dark theme is set via such an extension (as it is in our case), the incorrect theme might be briefly visible. To fix this the hook was completely redesigned: It's now officially a compartment hook. The extension(s) now has to be created inside the component and passed to the hook. The hook will return a compartment extension and a function to update said compartment. This allows the component to compute the extensions via useMemo and update the compartment via useEffect. I moved the hook into the shared CodeMirror editor module because it's API seems generic enough now.

The "scroll into view" logic was a bit inconsistent: Because it wasn't clear which lines are actually visible, the effect would be configured with "nearest" to ensure that the line was definitely visible. But that leads to an inconsistent behavior. While I'm not sure whether that's the current way to do it, it's actually possible to determine whether or not a line is visible by getting its vertical position and compare it against the scroll containers scroll position. Now a selected line is always centered when it is not visible (whether or not that behavior is the best UX is a different question)

Finally there is an issue when navigating back to a file that has a line selected that is outside of the range of lines of the current file. That's because the position change triggers an update before the new file content is loaded. A simple workaround is to verify that the selected line is within the range of the current document. (long term I'd like to find another solution)

I originally also included a change that would change how browser history entries are added when shift+clicking a line but such a behavior change needs more thought.

https://user-images.githubusercontent.com/179026/179524456-381a617e-1a43-46c2-8f02-26d7619cf74a.mp4

Test plan

Manual testing.

App preview:

Check out the client app preview documentation to learn more.

Merge request reports

Loading