Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Inline editor background doesn't extend all the way to right when scrolling #1240

Closed
peterflynn opened this issue Jul 12, 2012 · 4 comments
Closed
Assignees

Comments

@peterflynn
Copy link
Member

  1. Open the "citrus completed" smokes folder
  2. Make your Brackets window ~1100px wide or less
  3. Open inline editor on <body>
  4. Scroll to the right

Result: When you scroll way to the right, there's a gap between the background of the inline editor and the background of the rule list. The narrower the window, the bigger the gap.

Expected: No gap. Worked fine before the CodeMirror merge.

This only repros after the Sprint 11 CodeMirror merge from upstream.

@ghost ghost assigned peterflynn Jul 12, 2012
@peterflynn
Copy link
Member Author

CodeMirror changed the way editors are propped open to the max line width, so presumably that's interfering with how MultiRangeInlineEditor is calculating the width of the inline widget that it creates.

@peterflynn
Copy link
Member Author

So here's a summary... CodeMirror changed the way it props its width open: the "lineSpace" div used to have a fixed width set on it based on the length of the text's longest line. Now it's a regular div, and long lines overflow so its offsetWidth never exceeds the width of the viewport.

The "scroller" div still shows a horizontal scrollbar for two reasons:

  • Since "lineSpace" is its descendant, its overflow contributes to "scroller"'s scrollWidth. Any long lines that are currently visible will thus keep the scrollbar on.
  • CodeMirror added a new div ("widthForcer") that is always positioned far enough to the right to maintain the horizontal scrollbar as needed, regardless of which lines are currently rendered. This div looks a lot like a second cursor div (same className), but is actually not used for the cursor at all.

The logic in MultiRangeInlineEditor uses lineSpace's offsetWidth to size the inline widget. Hopefully it can just use scrollWidth instead... if not, we can probably use something like the left pos of widthForcer.

We have other logic that depends on the $.offset() of lineSpace. I think that's still fine, since the change from fixed-width propping to overflow: visible doesn't affect horizontal positioning at all.

@peterflynn
Copy link
Member Author

p.s., the CodeMirror change was originally prompted by a bug NJ filed: marijnh/CodeMirror2#550

peterflynn added a commit that referenced this issue Jul 17, 2012
right when scrolling) - CodeMirror changed the way it props its width open
such that lineSpace's width is no longer explicitly set to the max width
of all the lines (lines wider than the viewport simply overflow it). We were
checking this width to decide how wide inline editors must be. However,
lineSpace's _content_ is still as wide as the max line (due to the pos of
the new widthForcer div) so we can use lineSpace's scrollWidth and still get
an accurate number.
redmunds added a commit that referenced this issue Jul 17, 2012
Fix issue #1240 (Inline editor background width)
@redmunds
Copy link
Contributor

Verified fix. Closing.

tvoliter added a commit that referenced this issue Jul 18, 2012
* origin/master: (23 commits)
  - code review fixes - removed _removePopup() function since complexity of having this helper function isn't worth the savings of reusing the "index" var. I don't except their to be many visible popups at one time, so computing the index more than once isn't costly
  fixes issues #1270 #1271 #1272
  Fix scope of beforeEach/afterEach
  Don't limit ourself in handling key events for html selection only. As we transit to js or css, we still need to dismiss the html code hints popup.
  Code review fixes. Revert menu focus change.
  use public document instead of private _codeMirror
  Fix issue #1240 (Inline editor background doesn't extend all the way to right when scrolling) - CodeMirror changed the way it props its width open such that lineSpace's width is no longer explicitly set to the max width of all the lines (lines wider than the viewport simply overflow it). We were checking this width to decide how wide inline editors must be. However, lineSpace's _content_ is still as wide as the max line (due to the pos of the new widthForcer div) so we can use lineSpace's scrollWidth and still get an accurate number.
  Move the long comment before the corresponding code.
  Code review fixes
  fix return key code
  code review fixes
  initial checkin
  Extract code hint query from tagInfo.tagName instead of extracting it directly from code mirror text using cursor position. Also fix an offset issue in HTMLUtils when the cursor is after white spaces and before a left angle bracket.
  Alternative fix for issue #1028. Add new file in working set, but keep it selected in the project tree since that is where the user created and named the file.
  remove debug code
  fix async loading of extensions. init jasmine after extension tests are loaded
  fix module loading for SpecRunnerUtils
  Additional comments in Menus.js for sections
  Adding function comment blocks to Menus.js
  remove empty menu section test
  ...

Conflicts:
	src/widgets/PopUpManager.js
tvoliter added a commit that referenced this issue Jul 31, 2012
* 'master' of github.com:adobe/brackets: (94 commits)
  Don't limit ourself in handling key events for html selection only. As we transit to js or css, we still need to dismiss the html code hints popup.
  Code review fixes. Revert menu focus change.
  use public document instead of private _codeMirror
  Fix issue #1240 (Inline editor background doesn't extend all the way to right when scrolling) - CodeMirror changed the way it props its width open such that lineSpace's width is no longer explicitly set to the max width of all the lines (lines wider than the viewport simply overflow it). We were checking this width to decide how wide inline editors must be. However, lineSpace's _content_ is still as wide as the max line (due to the pos of the new widthForcer div) so we can use lineSpace's scrollWidth and still get an accurate number.
  Move the long comment before the corresponding code.
  Code review fixes
  fix return key code
  Bug fixes
  code review fixes
  Implement generic pop-up Esc key handling
  initial checkin
  Extract code hint query from tagInfo.tagName instead of extracting it directly from code mirror text using cursor position. Also fix an offset issue in HTMLUtils when the cursor is after white spaces and before a left angle bracket.
  Alternative fix for issue #1028. Add new file in working set, but keep it selected in the project tree since that is where the user created and named the file.
  Implement remove all widgets
  remove debug code
  fix async loading of extensions. init jasmine after extension tests are loaded
  Add escape key handling for menus and inline widgets
  Update CodeMirror SHA
  Clarify clickDialogButton async scenarios
  fix module loading for SpecRunnerUtils
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants