Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ext-dnd: options for drop marker offset #693

Merged
merged 2 commits into from
Feb 28, 2017

Conversation

evil-shrike
Copy link
Contributor

Hard-coded offset for drop marker was replaced by usage of values from extension's options (dropMarkerOffset/dropMarkerOffsetBefore/dropMarkerOffsetAfter).

…pMarkerOffsetBefore/dropMarkerOffsetAfter)
Copy link
Owner

@mar10 mar10 left a comment

Choose a reason for hiding this comment

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

Thanks!
Would you mind to apply this changes to ext-dnd5 as well.

break;
case "after":
markerAt = "bottom";
markerOffsetX -= 16;
markerOffsetX += (dndOpt.dropMarkerOffsetAfter || 0);
Copy link
Owner

Choose a reason for hiding this comment

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

This offset will always be the same for 'after' and 'before' so I suggest to combine it into one single option.
Also we could use two positive defaults:
dropMarkerOffsetX: 8
dropMarkerOffsetIndentX: 16

before/after offset will be dropMarkerOffsetX
otherwise dropMarkerOffsetX + dropMarkerOffsetIndentX

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about positive values, but if before/after offsets are evaluating as a sum of two options (dropMarkerOffset+dropMarkerOffsetAfter) then it'd be unclear for me what to specify as dropMarkerOffsetAfter: negative value (-16) or positive (+16).
I think I'd make sense only if we'll have two separated options (offsetOver/offsetInsert) which won't be summed.

This offset will always be the same for 'after' and 'before' so I suggest to combine it into one single option.

Ok

Also we could use two positive defaults:
dropMarkerOffsetX: 8
dropMarkerOffsetIndentX: 16

I'm sorry, I don't understand, why 8/16? They are -24 and -16 currently, aren't them?

…fsetX/dropMarkerInsertOffsetX) instead of three

ext-dnd5: setting drop marker offset via options (the same as for etx-dnd)
@mar10 mar10 merged commit 2cf280c into mar10:master Feb 28, 2017
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.

2 participants