Closed Bug 1575911 Opened 5 years ago Closed 5 years ago

Inline Preview: The debug line no longer fills the width of the editor

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(firefox71 fixed)

RESOLVED FIXED
Firefox 71
Tracking Status
firefox71 --- fixed

People

(Reporter: dhyey35, Assigned: davidwalsh)

References

Details

(Whiteboard: [debugger-mvp])

Attachments

(4 files)

Attached image dbg-horizontal-issue.gif (deleted) —

When there are many inline previews on a single line the codemirror doc adds too much horizontal scrolling. Checkout the gif

Summary: Manage horizontal scrolling for inline preview → Inline Preview: The debug line no longer fills the width of the editor

Here is a hacky way to control the width of previews

diff --git a/devtools/client/debugger/src/components/Editor/InlinePreview.css b/devtools/client/debugger/src/components/Editor/InlinePreview.css
index b5e1aeb680e8..6e42586ebaad 100644
--- a/devtools/client/debugger/src/components/Editor/InlinePreview.css
+++ b/devtools/client/debugger/src/components/Editor/InlinePreview.css
@@ -5,6 +5,7 @@
 .inline-preview {
   position: relative;
   margin-top: calc(var(--theme-code-line-height) * -11px);
+  overflow: hidden;
 }

 .inline-preview-outer {
@@ -14,6 +15,7 @@
   border-radius: 3px;
   font-size: 10px;
   margin-right: 5px;
+  white-space: nowrap;
 }

 .inline-preview-label {
diff --git a/devtools/client/debugger/src/components/Editor/InlinePreviewRow.js b/devtools/client/debugger/src/components/Editor/InlinePreviewRow.js
index 47d3dec02e01..d97732bd9624 100644
--- a/devtools/client/debugger/src/components/Editor/InlinePreviewRow.js
+++ b/devtools/client/debugger/src/components/Editor/InlinePreviewRow.js
@@ -81,7 +81,9 @@ class InlinePreviewRow extends PureComponent<Props> {
     const left = this.getPreviewPosition(editor, line);
     if (!prevProps || this.lastLeft !== left) {
       this.lastLeft = left;
+      const sizerWdth = parseInt(dbg.getCM().display.sizer.style.minWidth);
       this.IPWidget.node.style.left = `${left}px`;
+      this.IPWidget.node.style.maxWidth = `${sizerWdth - left - 10}px`;
     }

We could improve this in a couple of ways:

  • pass in codemirror
  • get the display width via a codemirror api
  • we will need to re-render when the editor size changes such as sidebar widths or toolbox widths changing...

We could also find more proper codemirror APIs because left and maxWidth are definitely hacks...

Attached image CodeMirrorSizer.png (deleted) —

I think the issue is that the .Codemirror-Sizer element isn't accounting for the inline previews, and thus the debug line is getting cut off.

Whiteboard: [debugger-mvp]
Priority: -- → P2
Type: enhancement → defect
Attached image ChromeBrokenToo.png (deleted) —

I'm obsessed with figuring this out so I took a fresh look again today. A few notes:

  • The Codemirror-Sizer element dictates the length of the debug line via its calculated min-width style. This property appears to be calculated by (the number of characters on a line) * (character width). Switching to setBookmark helps visually to avoid a costly left calculation for the inline preview but its width has no bearing on CodeMirror's calculation -- CodeMirror doesn't account for it.

  • Dhyey's use of addLineWidget is the same as Chrome. I don't see any other tricks or CSS properties. They inject the line-widget with calculated left property.

  • Chrome is also broken due to inline preview (image attached)

I have a slightly off the wall idea. In devtools/client/debugger/src/components/Editor/InlinePreviews.js, before the editor.codeMirror.operation call, we:

  • Loop through each line, calculate the number of characters in the preview(s) names and values, then inject that number of invisible spaces at the end of the line using doc.replaceRange, padding it with extra spaces to make up for the CSS padding
  • Remove the spaces upon unmount.

No matter what we do, it's going to be unscientific, unfortunately. I also thought about injecting the inline previews as normal text via replaceRange but then we lose our styling abilities :/

yeah - i think the hack with replaceRange is fine. It is a fine work around until codemirror has internal support for it.

Assignee: nobody → dwalsh
Status: NEW → ASSIGNED
Pushed by dwalsh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/74b9647529d2
Use bookmarks for inline preview to fix debug line and improve performance by not using visible breakpoints r=jlast
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: