Skip to content

added -nofocus variant of all methods#20

Closed
dsbauer wants to merge 1 commit intoimhoffd:1.xfrom
dsbauer:1.x
Closed

added -nofocus variant of all methods#20
dsbauer wants to merge 1 commit intoimhoffd:1.xfrom
dsbauer:1.x

Conversation

@dsbauer
Copy link

@dsbauer dsbauer commented Apr 9, 2017

I needed to manipulate text selections without setting focus on the text element, and this seemed the best solution for adding that variant while remaining fully backward compatible.
Please consider merging this addition into the main package or implementing this feature by other means. Thanks!

@imhoffd
Copy link
Owner

imhoffd commented Apr 18, 2017

Thanks for the PR! I may take a look at re-implementing this with an options object as a second, optional parameter to each method.

As best practice for future PRs, please refrain from updating version numbers--the maintainer will release the software. 👍

@dsbauer
Copy link
Author

dsbauer commented Apr 18, 2017

A fair point; understood and agreed, thanks. I'll look forward to adopting your update when ready.

imhoffd added a commit that referenced this pull request Apr 29, 2017
alternative implementation of #20
@imhoffd
Copy link
Owner

imhoffd commented Apr 29, 2017

@dsbauer Does this suit your needs? https://github.com/dwieeb/jquery-textrange#options

1.4.0 is out.

@imhoffd imhoffd closed this Apr 29, 2017
@dsbauer
Copy link
Author

dsbauer commented May 2, 2017

Thanks for adding the nofocus option! Your solution ({...,nofocus:true}) works for me. It might be slightly cleaner conceptually to use implied {focus:true} as the default, with {focus:false} as the exception, but taste varies.
Depending on future additional options, another choice might be to allow multiple non-numeric string options after the first argument (method name), as in: textrange('set','nofocus',0,0) or textrange('get','nofocus','please','length'). But I'm good now, thanks.

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