Option to drag by fixed amounts/intervals

[adrianbj]

Thanks for a great script.
Just wondering if you would consider an option to snap dragging to a defined interval of pixels.

I am using this in conjunction with Ace editor and it would be great to be able to set the dragInterval to the same height as the Ace editor lineHeight setting.

I am sure there could be other use cases as well for this functionality.

Thanks!

[nathancahill]

Yes, I'd accept a PR for this idea.

[adrianbj]

OK, sounds good. I have already implemented here, although I did it outside the splitjs codebase using onDragEnd() so I'll need to rework it bit, but I'll take a look.

[nathancahill]

Look at the code that uses getOption to pass the option in, then the actual drag code is in the drag function here: https://github.com/nathancahill/Split.js/blob/master/src/split.js#L261

[adrianbj]

Not a proper PR yet, but since it's my first contribution here I wanted to make sure you are happy with how I have implemented this. Here are the changes I have made: adrianbj@34ea3ee

Obviously I need to do this in the file under src instead, but for quick testing this was easiest for now.

If you're happy with this, I'll redo as a proper PR. Do you want me to add to the docs as well? I assume you'll handle building/minifying?

Thanks!

[adrianbj]

Actually, I am using + this[bGutterSize] but should I use + this[aGutterSize] or does there need to be more logic here to know when to use which? In my testing they are always the same and half of the specified gutterSize.

[adrianbj]

I named the option snapInterval. Do you prefer this or dragInterval? Keep in mind my other recent suggestion for keyboard shortcuts for resizing in which case "drag" may be less appropriate.

[nathancahill]

Great! This looks good. You don't need to account for the gutter in the dragging math, that's handled later. I would say subtract half of the dragInterval, so that the gutter is more centered under the cursor. Change that line to this:

(Math.ceil(offset / snapInterval) * snapInterval) - (snapInterval / 2)

You can also get rid of this if statement and the snappedInterval variable:

if(offset!== snappedInterval) {

And just set offset directly instead:

offset = (Math.ceil(offset / snapInterval) * snapInterval) - (snapInterval / 2)

Please add the new option to the README, both in the markdown table and further down with the default value and description.

I'll handle building and minifying, so the only changes in the PR should be in src/split.js. Thanks!

[nathancahill]

Also I think dragInterval is a better name since we already use snap for snapping to minSize.

[nathancahill]

One more thing for your PR, add yourself to the AUTHORS.md file.

[adrianbj]

Thanks for the feedback.

I would say subtract half of the dragInterval, so that the gutter is more centered under the cursor. Change that line to this:
(Math.ceil(offset / snapInterval) * snapInterval) - (snapInterval / 2)

I don't think this works. It's actually half the gutter width that works for me - at least in my setup. Take a look at these screencasts.

This code:

        if (dragInterval !== 0) {
            var draggedInterval = (Math.ceil(offset / dragInterval) * dragInterval) + this[aGutterSize];
            if(offset!== dragInterval) {
                offset = draggedInterval;
            }
        }

results in:

But if I convert to what I think you are suggesting:

        if(snapInterval !== 0) {        
            offset = (Math.ceil(offset / dragInterval) * dragInterval) - (dragInterval / 2);
        }

I end up with this where the divider is not correctly positioned between the lines:

A simplified working version for me is:

        if(dragInterval !== 0) {
            offset = (Math.ceil(offset / dragInterval) * dragInterval) + this[aGutterSize];
        }

By the way, I am setting the new dragInterval value to match the ACEEditor lineHeight. I am also setting splitjs's minSize to this same value.

What do you think of the revised version still using the + this[aGutterSize] ? Or is there something I am missing in getting the - (dragInterval / 2) version to work as expected?

Thanks!

[nathancahill]

I see, your version matches the line heights. I was optimizing for keeping the cursor over the gutter. You can see the difference in the two (very nice) gifs you posted.

[adrianbj]

I see, your version matches the line heights. I was optimizing for keeping the cursor over the gutter. You can see the difference in the two (very nice) gifs you posted.

Right - I do now see why you wanted the mouse cursor over the gutter, but I think that the position of the gutter relative to the line height is more important - otherwise the end result is quite ineffective.

This seems to accommodate both needs relatively well:

if(dragInterval !== 0) {
     offset = (Math.ceil(offset / dragInterval) * dragInterval) - dragInterval + this[aGutterSize];
}

It still jumps obviously, but at least now each time it snaps it snaps into place under the cursor.

What do you think - would you be happy accepting this version?

[nathancahill]

Oh, here's the better option:

offset = Math.round(offset / dragInterval) * dragInterval

round instead of ceil. Keeps it centered and at the correct offsets for your rows.

[adrianbj]

That's pretty close, but as is, it looks like this:

It still needs half the gutter height to look like this:

Notice how the top of the gutter is now properly half way between row 4 and 5.

So final code for that is:

if(dragInterval !== 0) {
     offset = (Math.round(offset / dragInterval) * dragInterval) + this[aGutterSize];
}

Are you ok with this version?

[adrianbj]

Just to make it more obvious by adding code on line 4 - see how crowded it looks:

[nathancahill]

Sorry, I'm not trying to be difficult, just want to make sure this new option works all around and not just for this specific use case. In your example, the "correct" sizing will only apply to the first (top) element of the split. If you tried your example in the second (bottom) element, the sizing would be off because aGutterSize is for the top element.

I'm trying to come up with a solution that doesn't use aGutterSize or bGutterSize because those are specific to 1 of the 2 elements.

[adrianbj]

I don't think you're being difficult at all :)
I haven't looked into the core code that much so don't have a full understanding of the gutter stuff with respect to the bottom element, but I understand the issue you're getting at.

I guess I need to setup a working example with ACE editor instances in both elements so I can test properly.

[nathancahill]

I thought about this a bit more, and I think we'll need a second option besides dragInterval. I'm thinking gutterAlign, which can have the values 'start', 'end' or 'center'. This determines if the gutter sits before, after or in between the elements. The current behavior is 'center' and what you're needing for Ace is 'start' or 'end'.

Here's the 3 behaviors against a 20px background, with gutterSize: 20 and dragInterval: 20.

[nathancahill]

In 'start' and 'end' you can see how the gutter leads or trails the mouse. In 'center' it's directly under it.

[nathancahill]

Added this changes in PR #155. Please give it a try and see if that works for your use case.

[adrianbj]

That looks good and seems like a great solution - thanks for picking up on this and making it some much better / more versatile.

I tested the PR you just posted and the 'end' option works beautifully!

[nathancahill]

Merged and published as 1.5.3. Thanks!

[adrianbj]

Thank you very much!

[adrianbj]

Just a thought, but I wonder if it might be more semantically correct for the default value to be 1 instead of 0. An actual value of 0 would mean that dragging wouldn't move the split at all. 1 would be the equivalent of dragging naturally / smoothly one pixel at a time.

I don't feel strongly about it one way or the other, but thought I should bring it up given that my first code example used 0 so I put us on this path to start with :)

[nathancahill]

Yeah, good observation. Want to submit that as a PR? I think we just have to change the default and the if (dragInterval > 0) { check.

[adrianbj]

Sure, happy to. Just one question - what should happen if dragInterval is actually set to 0. Do you think it makes sense to have this as a way to prevent a gutter from being draggable at all?

Maybe this could actually be a useful option in some scenarios, especially when there are more than two elements, or as a way to lock dragging after some user interaction, etc.

What do you think?

[nathancahill]

I'd probably ignore 0. Seems a little unexpected to have different behavior like that, even if it is mathematically correct.

[nathancahill]

Published in v1.5.4.