Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

feat(tooltip): add configurable tooltip delay #713

Closed
wants to merge 4 commits into from
Closed

feat(tooltip): add configurable tooltip delay #713

wants to merge 4 commits into from

Conversation

paynecodes
Copy link
Contributor

Currently, there is no way to configure the tooltip delay timeout. This provides a basic attribute, md-tooltip-delay for doing so. Granted, this could be better following some discussion on how you feel about rolling in a config provider into the directive.

add md-tooltip-delay attribute binding
to accommodate a variable show delay
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

@paynecodes
Copy link
Contributor Author

Not sure what to do about the @googlebot message above. I have signed the CLA.

@Splaktar
Copy link
Member

Splaktar commented Dec 3, 2014

Looks like you authored the commit with another name and email address than what is registered with Github. Check your .gitconfig file in your home dir.

It may need to have name = jpdesigndev and not name = Jarrod Payne.
The email address should also be listed under your GitHub Settings

@ajoslin
Copy link
Contributor

ajoslin commented Jan 6, 2015

@jpdesigndev can you change the email as @Splaktar suggested so we can merge this?

@paynecodes
Copy link
Contributor Author

Sure thing. I'll just have to figure out how to do so.

edit Figured it out. Coming right up.

add md-tooltip-delay attribute binding
to accommodate a variable show delay
@paynecodes
Copy link
Contributor Author

@ajoslin Let me know if this works for you. Otherwise, I suppose I can create a clean PR. Thanks :)

@Splaktar
Copy link
Member

Looks like the CLA bot is happy now.

@paynecodes
Copy link
Contributor Author

Yes! I simply completed the original commit with work email or something weird. Anyways, I'd, of course, love for this to make it into the next release :)

@PaulMougel
Copy link
Contributor

Could you also add a quick documentation line for this feature? If I'm not mistaken, it should be added here.

@paynecodes
Copy link
Contributor Author

@PaulMougel Done. Dunno if you want the param defaults thing [mdTooltipDelay=400] that's documented in jsDoc, but I added it for clarity.

@paynecodes
Copy link
Contributor Author

I don't understand what's going on with the CLA bot on the last commit.

I am the author of that commit Jarrod Payne <jarrod.payne5@gmail.com>

Here's the commit on my repo.

@ajoslin ajoslin closed this in 755861c Jan 13, 2015
@ajoslin
Copy link
Contributor

ajoslin commented Jan 13, 2015

Added a unit test and merged! Thanks.

ajoslin pushed a commit that referenced this pull request Jan 13, 2015
@paynecodes
Copy link
Contributor Author

@ajoslin Awesome! Your welcome, and thank you.

@gkalpak
Copy link
Member

gkalpak commented Jan 14, 2015

@ajoslin: Are you sure the tests passed with your commit ? From looking at it, it seems incorrect.
E.g. you are using md-delay and md-tooltip-delay "interchangeably".
(And better test-coverage would also be nice 😄)

@ajoslin
Copy link
Contributor

ajoslin commented Jan 14, 2015

@gkalpak In the first commit, I had some changes that I accidentally did not amend into the commit before I pushed.

I subsequently force pushed with the second commit, which does work correctly.

@gkalpak
Copy link
Member

gkalpak commented Jan 14, 2015

Great !
Sorry for the noise :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants