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

Automatically scroll find selection into view #5861

Merged
merged 2 commits into from
Nov 6, 2013
Merged

Automatically scroll find selection into view #5861

merged 2 commits into from
Nov 6, 2013

Conversation

lkcampbell
Copy link
Contributor

This PR fixes issue #3063 by forcing the editor to always scroll a find selection into view.

@ghost ghost assigned redmunds Nov 5, 2013
@@ -144,6 +144,7 @@ define(function (require, exports, module) {
centerOptions = Editor.BOUNDARY_IGNORE_TOP;
}
editor.setSelection(cursor.from(), cursor.to(), true, centerOptions);
cm.scrollIntoView({from: cursor.from(), to: cursor.to()});
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this always shows selection horizontally, but it cancels the "center vertically" functionality. I see the same results with:

            editor.setSelection(cursor.from(), cursor.to(), false);
            //cm.scrollIntoView({from: cursor.from(), to: cursor.to()});

I think the problem may be that the Editor.centerOnCursor() function passes null as x position to this.setScroll(). Maybe that used to work and there was a change in CodeMirror. Try looking into cm.scrollIntoView() to see what it's doing.

@redmunds
Copy link
Contributor

redmunds commented Nov 6, 2013

Done with initial review.

@lkcampbell
Copy link
Contributor Author

@redmunds, good catch, I didn't even notice that behavior.

Fix submitted.

@redmunds
Copy link
Contributor

redmunds commented Nov 6, 2013

Looks good. Merging.

redmunds added a commit that referenced this pull request Nov 6, 2013
Automatically scroll find selection into view
@redmunds redmunds merged commit 9333acf into adobe:master Nov 6, 2013
@lkcampbell lkcampbell deleted the 3063-fix branch November 6, 2013 22:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants