Skip to content

Conversation

@Redsandro
Copy link
Contributor

@Redsandro Redsandro commented Feb 9, 2018

I have created a low impact delayed debounced scroll sync for the split editor.

This could probably be done better with some modules that allow queueing and animation in order to better catch missed events from rapid scrolling, but I know absolutely nothing about react, so I coded this moon-lander-style. That said, it's already infinitely better than nothing.

This would close #1355.

scroll

@Redsandro
Copy link
Contributor Author

Redsandro commented Feb 9, 2018

You can probably bind the scroll of one pane directly to the other by removing the debounce and removing the easing, but since these events are fired per (sub)pixel movement, you can rank up 1000+ events per second on a UHD screen and it didn't seem sane to do so. Feel free to improve or like it as is.

@kazup01 kazup01 added the awaiting review ❇️ Pull request is awaiting a review. label Feb 11, 2018
Copy link
Member

@sosukesuzuki sosukesuzuki left a comment

Choose a reason for hiding this comment

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

Thanks your for great contribution.
please confirm my review.:pray:

let f, t
const timer = setInterval(() => {
t = (s - i) / s
f = t < 0.5 ? 2 * t * t : -1 + (4 - 2 * t) * t
Copy link
Member

Choose a reason for hiding this comment

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

I want you to rename variables.
Because, it is difficult to understand meaning of s, t, f for me.

And, please refactor for conditional operator, like below.

f = t < 0.5
  ? 2 * t * t
  : -1 + (4 - 2 * t) * t

@kazup01 kazup01 added awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels Feb 19, 2018
@Redsandro
Copy link
Contributor Author

@sosukesuzuki Thanks for the review. I will address the issues, hopefully before or in the weekend.

@Redsandro
Copy link
Contributor Author

@sosukesuzuki Fixed and rebased. Ready when you are.

@Redsandro
Copy link
Contributor Author

Merged master. Please consider merging this so I can stop tracking upstream. 🙏

@Rokt33r Rokt33r self-requested a review February 21, 2018 21:22
@Rokt33r Rokt33r merged commit 646cded into BoostIO:master Feb 21, 2018
@Rokt33r Rokt33r added next release (v0.9.1) and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. labels Feb 21, 2018
@gramster
Copy link

Apparently this in in 0.10.0 but not working for me. Does it have to be enabled somehow?

@Rokt33r
Copy link
Member

Rokt33r commented Feb 26, 2018

@gramster Hmm.. there was a problem while merging lots of PRs. Could you create an issue about this?

@gramster
Copy link

#1586

I noticed it does work from preview window (markdown syncs), but not from markdown window (preview doesn't sync).

Redsandro added a commit to Redsandro/Boostnote that referenced this pull request Feb 26, 2018
Rokt33r added a commit that referenced this pull request Mar 5, 2018
Fixed bad merge; closes #1586; related to #1521
@nototono
Copy link

NOOB HERE
Please guys, how do i implent this to my Boost Note...
Please!

@Rokt33r
Copy link
Member

Rokt33r commented Mar 25, 2020

@nototono Scroll syncing has a bug now. It will be fixed soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Scroll sync between markdown & preview pane

6 participants