Make TextEditorView a component and improve toolbar styling/layout.
Review Request #13210 — Created Aug. 14, 2023 and submitted
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.
TextEditorViewis now backed by a
class, replacing the old
.CodeMirrornesting. 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
.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
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
Tested the following in Chrome, Safari, and Firefox with both plain text
- 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.
Can we swap these?
tagNamein alphabetical order.
Revision 2 (+280 -130)
Checks run (2 succeeded)