Make TextEditorView a component and improve toolbar styling/layout.

Review Request #13210 — Created Aug. 14, 2023 and submitted

Information

Review Board
release-6.x

Reviewers

This change addresses some problems inherent to the way the formatting
toolbar was placed in CodeMirror's DOM, and uses the opportunity to turn
this all into a documented component and tweak styling.

TextEditorView is now backed by a .rb-c-text-editor CSS component
class, replacing the old .text-editor and plain textarea /
.CodeMirror nesting. This gives us a solid structure for the
component that we can build upon. Some scattered rules were cleaned up
in the process, helping reduce how widespread our styling was.

The formatting toolbar now lives within .rbc-text-editor, and not
within .CodeMirror. This addresses problems with spacing and layout.
By being placed in .CodeMirror, we ended up overlapping a horizontal
scrollbar, and this ends up causing some layout wonkiness when stressing
the text editor (such as filling the comment dialog with enough text to
force scrolling).

To provide some visual separation, the background of the toolbar is now
a light grey, with 2em of spacing above it (technically, an offset of
2em at the bottom of the CodeMirror scroll container). This gives enough
breathing room to write without the toolbar being too far away.

In the inline editors with growing text fields, we get those 2em of
space at all times. In the comment dialog, it's allowed to reach down to
the top of the toolbar, so that the scrollbar is flush against it. This
mirrors the behaviors prior to adding the toolbar.

To reduce visual noise and focus on the buttons, the vertical separators
in the toolbar have been replaced with additional spacing. The hit areas
for the buttons are also a bit larger now, making them easier to press
on mobile.

Tested the following in Chrome, Safari, and Firefox with both plain text
and Markdown:

  • Inline editors in the review request field
  • Inline editors in the review dialog
  • Comment dialog

Each was tested with small amounts of text and many pages of text, to check
for any scrolling or padding issues.

Made sure that the ancient styles that were removed weren't actually being
used and that their removal didn't affect any display or usage.

Summary ID
Make TextEditorView a component and improve toolbar styling/layout.
This change addresses some problems inherent to the way the formatting toolbar was placed in CodeMirror's DOM, and uses the opportunity to turn this all into a documented component and tweak styling. `TextEditorView` is now backed by a `.rb-c-text-editor` CSS component class, replacing the old `.text-editor` and plain `textarea` / `.CodeMirror` nesting. This gives us a solid structure for the component that we can build upon. Some scattered rules were cleaned up in the process, helping reduce how widespread our styling was. The formatting toolbar now lives within `.rbc-text-editor`, and not within `.CodeMirror`. This addresses problems with spacing and layout. By being placed in `.CodeMirror`, we ended up overlapping a horizontal scrollbar, and this ends up causing some layout wonkiness when stressing the text editor (such as filling the comment dialog with enough text to force scrolling). To provide some visual separation, the background of the toolbar is now a light grey, with 2em of spacing above it (technically, an offset of 2em at the bottom of the CodeMirror scroll container). This gives enough breathing room to write without the toolbar being too far away. In the inline editors with growing text fields, we get those 2em of space at all times. In the comment dialog, it's allowed to reach down to the top of the toolbar, so that the scrollbar is flush against it. This mirrors the behaviors prior to adding the toolbar. To reduce visual noise and focus on the buttons, the vertical separators in the toolbar have been replaced with additional spacing. The hit areas for the buttons are also a bit larger now, making them easier to press on mobile.
a26af4ef758d21ea421a6298178ed86839b9170d

Description From Last Updated

Can we swap these?

daviddavid
maubin
  1. Ship It!
  2. 
      
david
  1. 
      
  2. Show all issues

    Can we swap these?

  3. 
      
chipx86
david
  1. Ship It!
  2. 
      
maubin
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (90529c9)
Loading...