Skip to content

Reverted a change breaking the reverse flag in dwr.util.addOptions #6

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rogwes
Copy link

@rogwes rogwes commented May 10, 2018

Reverted a change breaking the reverse flag in dwr.util.addOptions(selectid, map, reverse) call.

In commit 101a341 a change was introduced in dwr.util.addOptions to instead of checking if arg3 is falsy (null or false or something else that can be interpreted as false) it now checks if arg3 is null. The effect of this will is that calls like addOptions(selectid, map, false) will be interpreted the same as addOptions(selectid, map, true).thus ignoring the flag value.

@mikewse
Copy link
Contributor

mikewse commented May 28, 2018

Hey @rogwes, thanks for finding and reporting this!
I'll ask you to bear with us a little while longer. We are currently working on setting up cla-bot for our repos and once that's up and running it will post to this PR and ask you to confirm.
In the meantime please update this PR and focus it on the actual bugfix. You can put all the whitespace changes in a separate PR if you desire.

@rogwes
Copy link
Author

rogwes commented May 29, 2018

Oh, I'm so sorry about the white space changes, that was my IDE setting that did that. I'll fix that.

@rogwes rogwes force-pushed the util-js-addoptions-reverse-fix branch from f51a86c to 7bf83a3 Compare May 29, 2018 04:20
@rogwes
Copy link
Author

rogwes commented May 29, 2018

Whitespaces fixed, just let me know when you need my action again.

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