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

Trigger options key code fix (fix #1285) #1286

Closed
wants to merge 2 commits into from

Conversation

igor-nabebin
Copy link

@igor-nabebin igor-nabebin commented Jul 31, 2019

Allowing to specify keyCode option while calling trigger method, throwing warning if both keyCode option and event modifier are specified (keyCode has higher priority so modifier will be ignored in this case).

Related issue #1285

P.S. Failed CI test is my fault, commit with tests must not be tested separately from the first one.

@lmiller1990
Copy link
Member

lmiller1990 commented Jan 3, 2020

Hi @igor-nabebin , sorry no-one was able to review your code. Are you still interested in getting this one merged up? If you can get the tests passing and resolve, I can give you a review.

If not, I can probably do those for you - let me know.

@lmiller1990 lmiller1990 self-assigned this Jan 3, 2020
@nabebin
Copy link

nabebin commented Jan 27, 2020

Hi, @lmiller1990
I'm still interested in merging this PR and I think I also could make tests pass. The only problem is I've lost access to my account and I can't update the branch, so I'm afraid I'll have to open another PR and this one could be closed.

@lmiller1990
Copy link
Member

@Lamberthassel I can push some commits to your branch resolving your conflicts if you are busy. Alternatively if you have time to reopen a fresh PR that would be fine too. Your choice! Let me know.

@nabebin
Copy link

nabebin commented Feb 3, 2020

Ah, I've just found out it has already been fixed few months ago.
#1378

I think the PR could be closed.

@dobromir-hristov
Copy link
Contributor

Cool, let us know if you find something related that seems broken :)

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.

4 participants