Skip to content
Snippets Groups Projects

codemirror file view: Make view non-editable (remove caret)

Merged Warren Gifford requested to merge fkling/cm-file-view-disable-caret into main

Created by: fkling

The original reason for making the view editable (but readonly) was to make it focusable so that Mod-f would trigger CodeMirror's search functionality.

The side effect of this was that the view would show a real caret, which is not desirable at this point. Furthermore it changes how the view can be scrolled via the keyboard.

PR #40232 tried to solve the caret problem by simply hiding it, but the keyboard scroll issues remained.

This PR makes the view non-editable (which solves caret and keyboard scroll issues) but keeps it focusable (to keep Mod-f working) by adding tabindex=0 to its content element.

Without the CSS changes it wouldn't be apparent that/when the blob view has focus.

CodeMirror behavior:

https://user-images.githubusercontent.com/179026/184348757-1d093d08-c4a0-40a5-86c2-60a22e6bf1ae.mp4

Old blob view behavior (before/after): Before, clicking on the file didn't show the focus ring, the focus ring was only shown when a key (for scrolling?) was pressed. With these changes the focus ring is also always shown when the element has focus.

Note: If this change is not acceptable, we can remove it and tackle the focus in another PR.

https://user-images.githubusercontent.com/179026/184348740-d13b3825-e07c-4436-bbaf-15f752eeb521.mp4

Test plan

  • Enable CodeMirror file view
  • Open a file and click inside it.
    • Focus ring should appear
    • Pressing up/down arrow, space, mod-up/dow arrow should scroll the file view
    • Pressing Mod-f opens CodeMirror's search functionality

App preview:

Check out the client app preview documentation to learn more.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Created by: umpox

    Review: Approved

    Nice!

    With these changes the focus ring is also always shown when the element has focus.

    Tbh I think this is actually preferable to it suddenly appearing when a key is finally pressed. At least now it gives an indication that the scrolling behavior on the page will have changed

  • Merged by: fkling at 2022-08-12 12:39:01 UTC

Please register or sign in to reply
Loading