Skip to content
This repository has been archived by the owner on Dec 1, 2023. It is now read-only.

Scroll cursor back into view when it goes out of bounds #34

Merged
merged 10 commits into from
May 13, 2016
Merged

Scroll cursor back into view when it goes out of bounds #34

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
154e5e1
Add JS method for finding caret position to help scrolling
jesiegel1 Mar 16, 2016
55396c1
fix scrolling bug when deleting all characters on a line
jesiegel1 Mar 16, 2016
5dbbf81
Modify css to fix scrolling bug
jesiegel1 Mar 16, 2016
d3bf5b1
Fix scrollView offset and JS caret methods
jesiegel1 Mar 17, 2016
ae7c2fe
add method to wrap all text nodes in divs, add new-line boolean
jesiegel1 Apr 17, 2016
50597b9
update scrollView contentOffset when caret above upper scrollView bounds
jesiegel1 Apr 17, 2016
c6c5be5
fixes bug when new line parent node is not 'div'
jesiegel1 Apr 18, 2016
13ebbbf
clean up and bug fixes for setting scrollView offset
jesiegel1 Apr 21, 2016
7c59a46
fixes
jesiegel1 May 4, 2016
d39d4c3
indentation fix
jesiegel1 May 4, 2016
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
7 changes: 7 additions & 0 deletions RichEditorView.xcodeproj/project.xcworkspace/contents.xcworkspacedata
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 24 additions & 0 deletions RichEditorView/Assets/editor/rich_editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,3 +372,27 @@ RE.getSelectedHref = function() {

return href ? href : null;
};

/* retrieve caret vertical position */

RE.getCaretPosition = function() {
var x=0, y=0;
var sel=window.getSelection();
if (sel.rangeCount) {
var range = sel.getRangeAt(0);
var needsWorkAround = (range.startContainer.nodeName.toLowerCase() == 'div' && range.startOffset == 0);
Copy link
Owner

Choose a reason for hiding this comment

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

Could you add a comment or something about what this workaround is for and what it's doing?
Like, why is it needed? What is it returning instead of the normal case?

if (needsWorkAround) {
x=range.startContainer.offsetLeft;
y=range.startContainer.offsetTop + range.startContainer.clientHeight;
} else {
if (range.getClientRects) {
var rects=range.getClientRects();
if (rects.length > 0) {
x = rects[0].left;
y = rects[0].bottom;
}
}
}
}
return y;
};
13 changes: 11 additions & 2 deletions RichEditorView/Assets/editor/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,24 @@
@charset "UTF-8";

html {
height: 100%;
min-height: 100%;
}

body {
min-height: 100%;
overflow: hidden;
overflow: auto;
}

body, h1, p {
margin: 0;
}

div {
display: block;
position: static;
min-height: 28px;
Copy link
Owner

Choose a reason for hiding this comment

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

What is this 28px about? Is it necessary? I actually don't know the implications of much of this CSS that's being added. I hope it doesn't change the way it looks for people who are already using it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I made the <div> height the same height as the line-height for #editor

}

#editor {
-webkit-user-select: auto !important;
-webkit-tap-highlight-color: rgba(0, 0, 0, 0);
Expand All @@ -47,6 +53,9 @@ body, h1, p {

#editor {
min-height: 100%;
height: 100%;
overflow: auto;
display: block;
font-family: 'HelveticaNeue';
Copy link
Owner

Choose a reason for hiding this comment

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

it looks like it wasn't your fault, but could you get this indention to be the same?

color:#666666;
line-height:28px;
Expand Down
36 changes: 36 additions & 0 deletions RichEditorView/Classes/RichEditorView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,12 @@ extension RichEditorView {
return runJS("RE.rangeOrCaretSelectionExists();") == "true" ? true : false
}

/** Returns caret vertical position */

public func getCaretPosition() -> String {
return runJS("RE.getCaretPosition();")
}

/**
* If the current selection's parent is an anchor tag, get the href.
* @returns nil if href is empty, otherwise a non-empty String
Expand Down Expand Up @@ -439,6 +445,35 @@ extension RichEditorView: UIGestureRecognizerDelegate {
// MARK: - Utilities
extension RichEditorView {

/** Sets content offset for scrollView based on caret location **/

private func fixScrollView(editor: RichEditorView) {
Copy link
Owner

Choose a reason for hiding this comment

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

This is an instance method, you don't need to pass in the RichEditorView.

Also fixScrollView() is pretty vague. Maybe something like scrollCursorIntoView() ? I don't know, maybe there's something better, but it'd be nice to have something more clear about what it does.


var scrollView = editor.webView.scrollView

var contentHeight: CGFloat?
let htmlHeight = editor.runJS("document.getElementById('editor').clientHeight;")
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally a good practice to wrap the JS implementation details with a public accessor method on the RE namespace IMHO

Copy link
Owner

Choose a reason for hiding this comment

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

I agree. Even in this case, since document.getElementById('editor').clientHeight is literally the same as RE.editor, I might not be too opposed to just RE.editor.clientHeight (I'm not sure how I'm doing it in other places.)

Copy link
Collaborator Author

@jesiegel1 jesiegel1 Apr 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

did it that way to match: https://github.com/jesiegel1/RichEditorView/blob/master/RichEditorView/Classes/RichEditorView.swift#L176

if let n = NSNumberFormatter().numberFromString(htmlHeight) {
Copy link
Owner

Choose a reason for hiding this comment

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

In Swift you can just do Float(htmlHeight) without having to get an NSNumberFormatter involved

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cjwirth http://stackoverflow.com/questions/27595799/convert-string-to-cgfloat-in-swift

let floatValue = CGFloat(n)
contentHeight = floatValue
} else {
contentHeight = scrollView.frame.height
}
scrollView.contentSize = CGSizeMake(scrollView.frame.width, contentHeight!)

let caretPosition: String = editor.getCaretPosition()

if let caretPositionNumeric = NSNumberFormatter().numberFromString(caretPosition) {
let caretFloat = CGFloat(caretPositionNumeric) - scrollView.contentOffset.y
if caretFloat >= (scrollView.bounds.size.height) {
let bottomOffset = CGPointMake(0, (caretFloat + scrollView.contentOffset.y) - scrollView.bounds.size.height + scrollView.contentInset.bottom
)
scrollView.setContentOffset(bottomOffset, animated: true)
}
}

}

/**
Runs some JavaScript on the UIWebView and returns the result
If there is no result, returns an empty string
Expand Down Expand Up @@ -513,6 +548,7 @@ extension RichEditorView {
updateHeight()
}
else if method.hasPrefix("input") {
fixScrollView(self)
let content = runJS("RE.getHtml()")
contentHTML = content
updateHeight()
Expand Down
4 changes: 2 additions & 2 deletions RichEditorViewSample/RichEditorViewSample/KeyboardManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ class KeyboardManager: NSObject {
Starts monitoring for keyboard notifications in order to show/hide the toolbar
*/
func beginMonitoring() {
NSNotificationCenter.defaultCenter().addObserver(self, selector: #selector(KeyboardManager.keyboardWillShowOrHide(_:)), name: UIKeyboardWillShowNotification, object: nil)
NSNotificationCenter.defaultCenter().addObserver(self, selector: #selector(KeyboardManager.keyboardWillShowOrHide(_:)), name: UIKeyboardWillHideNotification, object: nil)
NSNotificationCenter.defaultCenter().addObserver(self, selector: Selector("KeyboardManager.keyboardWillShowOrHide:"), name: UIKeyboardWillShowNotification, object: nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

The project is already using Swift 2.2 selector syntax, doesn't it make sense to use here too?

Copy link
Owner

Choose a reason for hiding this comment

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

Nice catch. Now that Xcode 7.3 official is out, I think we should migrate everything to being Swift 2.2 only. Although -- this file is just in the sample project. Do you think we should make a major version change if we have changes which break in Xcode 7.2 but work in 7.3? I feel like I want to say yes...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea, I simply switched it back since 7.3 wasn't out and switching it back every time i updated a branch was getting annoying

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah it would be nice to have these in Xcode 7.3 form

NSNotificationCenter.defaultCenter().addObserver(self, selector: Selector("KeyboardManager.keyboardWillShowOrHide:"), name: UIKeyboardWillHideNotification, object: nil)
}

/**
Expand Down