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

Remove the use of the viewport hack #7118

Merged
merged 1 commit into from
Mar 7, 2014
Merged

Remove the use of the viewport hack #7118

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 45 additions & 64 deletions src/view/ViewCommandHandlers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,16 @@
define(function (require, exports, module) {
"use strict";

var Commands = require("command/Commands"),
CommandManager = require("command/CommandManager"),
KeyBindingManager = require("command/KeyBindingManager"),
Strings = require("strings"),
ProjectManager = require("project/ProjectManager"),
EditorManager = require("editor/EditorManager"),
PreferencesManager = require("preferences/PreferencesManager"),
DocumentManager = require("document/DocumentManager"),
AppInit = require("utils/AppInit");
var Commands = require("command/Commands"),
CommandManager = require("command/CommandManager"),
KeyBindingManager = require("command/KeyBindingManager"),
Strings = require("strings"),
ProjectManager = require("project/ProjectManager"),
EditorManager = require("editor/EditorManager"),
PreferencesManager = require("preferences/PreferencesManager"),
DocumentManager = require("document/DocumentManager"),
AppInit = require("utils/AppInit");


/**
* @const
Expand Down Expand Up @@ -95,17 +96,14 @@ define(function (require, exports, module) {
/**
* @private
* Sets the font size and restores the scroll position as best as possible.
* TODO: Remove the viewportTop hack and direclty use scrollPos.y once #3115 is fixed.
* @param {string} fontSizeStyle A string with the font size and the size unit
* @param {string} lineHeightStyle A string with the line height and a the size unit
*/
function _setSizeAndRestoreScroll(fontSizeStyle, lineHeightStyle) {
var editor = EditorManager.getCurrentFullEditor(),
oldWidth = editor._codeMirror.defaultCharWidth(),
oldHeight = editor.getTextHeight(),
scrollPos = editor.getScrollPos(),
viewportTop = $(".CodeMirror-lines", editor.getRootElement()).parent().position().top,
scrollTop = scrollPos.y - viewportTop;
var editor = EditorManager.getCurrentFullEditor(),
oldWidth = editor._codeMirror.defaultCharWidth(),
oldHeight = editor.getTextHeight(),
scrollPos = editor.getScrollPos();

// It's necessary to inject a new rule to address all editors.
_removeDynamicFontSize();
Expand All @@ -117,17 +115,17 @@ define(function (require, exports, module) {

editor.refreshAll();

// Calculate the new scroll based on the old font sizes and scroll position
var newWidth = editor._codeMirror.defaultCharWidth(),
newHeight = editor.getTextHeight(),
deltaX = scrollPos.x / oldWidth,
deltaY = scrollTop / oldHeight,
scrollPosX = scrollPos.x + Math.round(deltaX * (newWidth - oldWidth)),
scrollPosY = scrollPos.y + Math.round(deltaY * (newHeight - oldHeight));

// Scroll the document back to its original position, but not on the first load since the position
// was saved with the new height and already been restored.
if (_fontSizePrefsLoaded) {
// Calculate the new scroll based on the old font sizes and scroll position
var newWidth = editor._codeMirror.defaultCharWidth(),
newHeight = editor.getTextHeight(),
deltaX = scrollPos.x / oldWidth,
deltaY = scrollPos.y / oldHeight,
scrollPosX = scrollPos.x + Math.round(deltaX * (newWidth - oldWidth)),
scrollPosY = scrollPos.y + Math.round(deltaY * (newHeight - oldHeight));

editor.setScrollPos(scrollPosX, scrollPosY);
}
}
Expand Down Expand Up @@ -241,19 +239,17 @@ define(function (require, exports, module) {
* @param {number} textHeight
* @param {number} scrollTop
* @param {number} editorHeight
* @param {number} viewportFrom
* @return {{first: number, last: number}}
*/
function _getLinesInView(textHeight, scrollTop, editorHeight, viewportFrom) {
function _getLinesInView(textHeight, scrollTop, editorHeight) {
var scrolledTop = scrollTop / textHeight,
scrolledBottom = (scrollTop + editorHeight) / textHeight;

// Subtract a line from both for zero-based index. Also adjust last line
// to round inward to show a whole lines.
var firstLine = Math.ceil(scrolledTop) - 1,
lastLine = Math.floor(scrolledBottom) - 2;
// Adjust last line to round inward to show a whole lines.
var firstLine = Math.ceil(scrolledTop),
lastLine = Math.floor(scrolledBottom) - 1;

return { first: viewportFrom + firstLine, last: viewportFrom + lastLine };
return { first: firstLine, last: lastLine };
}

/**
Expand All @@ -268,35 +264,26 @@ define(function (require, exports, module) {
hasSelecction = editor.hasSelection(),
inlineEditors = editor.getInlineWidgets(),
scrollInfo = editor._codeMirror.getScrollInfo(),
viewportFrom = editor._codeMirror.getViewport().from,
paddingTop = editor._getLineSpaceElement().offsetTop,
viewportTop = $(".CodeMirror-lines", editor.getRootElement()).parent().position().top,
editorHeight = scrollInfo.clientHeight;

// To make it snap better to lines and dont cover the cursor when the scroll is lower than the top padding,
// we make it start direclty from the top padding
var scrolledTop = scrollInfo.top < paddingTop && direction > 0 ? paddingTop : scrollInfo.top;

// CodeMirror has a strange behaviour when it comes to calculate the height of the not rendered lines,
// so instead, we calculate the amount of hidden rendered lines at top and add it to the first rendered line.
var scrollTop = scrolledTop - viewportTop,
linesInView = _getLinesInView(textHeight, scrollTop, editorHeight, viewportFrom);
editorHeight = scrollInfo.clientHeight,
scrollTop = scrollInfo.top - paddingTop,
linesInView = _getLinesInView(textHeight, scrollTop, editorHeight),
inlinesHeight = 0;

// Go through all the editors and reduce the scroll top and editor height to recalculate the lines in view
var line, total;
var line, coords;
inlineEditors.forEach(function (inlineEditor) {
line = editor._getInlineWidgetLineNumber(inlineEditor);
total = inlineEditor.info.height / textHeight;
line = editor._getInlineWidgetLineNumber(inlineEditor);
coords = editor._codeMirror.charCoords({line: line, ch: 0}, "local");

if (line >= viewportFrom) {
if (line < linesInView.first) {
scrollTop -= inlineEditor.info.height;
linesInView = _getLinesInView(textHeight, scrollTop, editorHeight, viewportFrom);

} else if (line + total < linesInView.last) {
editorHeight -= inlineEditor.info.height;
linesInView = _getLinesInView(textHeight, scrollTop, editorHeight, viewportFrom);
}
if (coords.top < scrollInfo.top) {
scrollTop -= inlineEditor.info.height;
linesInView = _getLinesInView(textHeight, scrollTop, editorHeight);
inlinesHeight += inlineEditor.info.height;

} else if (coords.top + inlineEditor.info.height < scrollInfo.top + editorHeight) {
editorHeight -= inlineEditor.info.height;
linesInView = _getLinesInView(textHeight, scrollTop, editorHeight);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it doesn't use linesInView inside the forEach, I could just calculate the new scrollTop and editorHeight and call _getLinesInView(textHeight, scrollTop, editorHeight); just once after this for each.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explanation. I did try to understand the logic here and did verify and test scrolling with several inline editors and watched the cursor moving to next line when the previous line is moving out of view.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hehe, wasn't trying to explain it, but mentioning that the calculations here could be simplified. The idea of this for each is to remove the height of the inline editors from the scroll position and the editor height. After that is done I can just divide the left over scroll and height by the text height and get the correct first and last line in view.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I misunderstand you. Please create another pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done #7120

});

Expand All @@ -317,15 +304,9 @@ define(function (require, exports, module) {
}
}

// If there are inline editors just add/remove 1 line to the scroll top.
if (inlineEditors.length) {
editor.setScrollPos(scrollInfo.left, scrolledTop + (textHeight * direction));

// If there arent, we can make it snap to the line.
} else {
var lines = linesInView.first - viewportFrom + direction + 1;
editor.setScrollPos(scrollInfo.left, viewportTop + (textHeight * lines));
}
// Scroll and make it snap to lines
var lines = linesInView.first + direction;
editor.setScrollPos(scrollInfo.left, (textHeight * lines) + paddingTop + inlinesHeight);
}

/** Scrolls one line up */
Expand Down