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

refactor(ui-sref): support mock-clicks/events with no data #767

Merged
merged 1 commit into from
Jan 16, 2014

Conversation

ajoslin
Copy link
Contributor

@ajoslin ajoslin commented Jan 16, 2014

Sorry, I accidentally pushed this to origin using my old angular-ui permissions. I meant to push to my fork. I figured it would be confusing now to delete the branch here because everyone got the notification now ...

This problem is found in an issue at ionic-team/ionic-framework#406 - a mock click event has to supply a which in the data, or ui-sref won't work.

This now inverses the logic to do nothing if e.which (or any event property) is the wrong value. So event data doesn't have to be supplied now. I added a test for this, too.

@ajoslin
Copy link
Contributor Author

ajoslin commented Jan 16, 2014

That works, too. Whatever is the preferred code-style for the repository maintainers, we'll see what they think.

@nateabele
Copy link
Contributor

Haha, I actually asked @wesleycho to explain what he meant. His style is preferred. My big concern is that this block of code has probably been rewritten half a dozen times, and detecting a non-modified left-click should not be this hard.

Can anyone think of some absolute point of reference that we could compare this to to make this this doesn't keep coming up over and over again?

Finally, not to be pedantic, but the commit message should be fix(uiSref). We're actually using ngdocs now, and I don't want it to yell at me. :-)

Oh, also! Tell the Ionic guys I said hi and they should come find me if they want to chat about UI Router.

@ajoslin
Copy link
Contributor Author

ajoslin commented Jan 16, 2014

Amended with your ideas.

Please be pedantic, it's important to have the right changelog :)

As to button detecting, https://github.com/segmentio/is-meta looks like it sort of does this (but not exactly). I just did component search click, as componentJS seems to have these goodies often. But of course we don't use component!

@nateabele
Copy link
Contributor

Please be pedantic, it's important to have the right changelog :)

Hahaha, okay then: we do directive component names camelBack, so ui-sref should be uiSref. Thanks for appreciating the pedantry. :-)

The updated patch looks fine. As soon as the commit message is tweaked, consider it merged.

@ajoslin
Copy link
Contributor Author

ajoslin commented Jan 16, 2014

Awesome! At least there are two people in this world who can appreciate code-pedantry now :-p

nateabele added a commit that referenced this pull request Jan 16, 2014
refactor(ui-sref): support mock-clicks/events with no data
@nateabele nateabele merged commit e3f3522 into master Jan 16, 2014
@adamdbradley
Copy link

Awesome, thanks @ajoslin for figuring this all out, and @nateabele for quickly merging it. We'll get Ionic updated with AngularUI Router v0.2.8, thanks.

@nateabele
Copy link
Contributor

@adamdbradley You bet. Glad it's working out well for you.

@nateabele nateabele deleted the sref-mock-click branch January 17, 2014 00:06
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.

3 participants