Add hover / blur callback events #262

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
@Peinkillar

Added hover and blur events to react on mouseover states.

Update ui/jquery.ui.selectmenu.js
Added hover and blur events to react on mouseover states.
@fnagel

This comment has been minimized.

Show comment Hide comment
@fnagel

fnagel Aug 30, 2012

Owner

Can you give ma an example what this fixes exactly? Perhaps with a demo fiddle (see #61)?

Owner

fnagel commented Aug 30, 2012

Can you give ma an example what this fixes exactly? Perhaps with a demo fiddle (see #61)?

@Peinkillar

This comment has been minimized.

Show comment Hide comment
@Peinkillar

Peinkillar Aug 30, 2012

It is not a fix but an enhancement. In my scenario in a selectmenu i have listed geometries which are drawn on a map like Google Maps for example. When you select an entry the correspondent geometry is selected but you will only notice which geometry it is when you have already selected one. With the suggested hover and blur event it would be possible to highlight these geometries on mouseover. I can imagine this would be helpful in several other cases or don't you think so? It would be only some lines of additional code.

It is not a fix but an enhancement. In my scenario in a selectmenu i have listed geometries which are drawn on a map like Google Maps for example. When you select an entry the correspondent geometry is selected but you will only notice which geometry it is when you have already selected one. With the suggested hover and blur event it would be possible to highlight these geometries on mouseover. I can imagine this would be helpful in several other cases or don't you think so? It would be only some lines of additional code.

@fnagel

This comment has been minimized.

Show comment Hide comment
@fnagel

fnagel Aug 30, 2012

Owner

Ahh, ok thanks for the explanation. Seems legit to me.

Why do you use public methods to fire the callbacks?

Owner

fnagel commented Aug 30, 2012

Ahh, ok thanks for the explanation. Seems legit to me.

Why do you use public methods to fire the callbacks?

@Peinkillar

This comment has been minimized.

Show comment Hide comment
@Peinkillar

Peinkillar Aug 31, 2012

What do you mean with public methods? The hover and blur methods are equivalent to your change or select methods. o you can declare the callback methods like this:

jQuery(this.featureSelect).selectmenu({
   ...,
  change: function () {},
  hover: function (e) {},
  blur: function (e) {}
});

What do you mean with public methods? The hover and blur methods are equivalent to your change or select methods. o you can declare the callback methods like this:

jQuery(this.featureSelect).selectmenu({
   ...,
  change: function () {},
  hover: function (e) {},
  blur: function (e) {}
});
@fnagel

This comment has been minimized.

Show comment Hide comment
@fnagel

fnagel Sep 3, 2012

No check for null here?

No check for null here?

@fnagel

This comment has been minimized.

Show comment Hide comment
@fnagel

fnagel Sep 3, 2012

Owner

Functions without _ prefix are public. Please see: http://wiki.jqueryui.com/w/page/12138135/Widget%20factory

So your hover and blur functions could be called by using myElement.selectmenu("blur").Why should this be possible? Afaics your current implementation would do nothing expect firing the callback.

hint: try typing "m" when not in a textfield for textformatting options!

Owner

fnagel commented Sep 3, 2012

Functions without _ prefix are public. Please see: http://wiki.jqueryui.com/w/page/12138135/Widget%20factory

So your hover and blur functions could be called by using myElement.selectmenu("blur").Why should this be possible? Afaics your current implementation would do nothing expect firing the callback.

hint: try typing "m" when not in a textfield for textformatting options!

Update ui/jquery.ui.selectmenu.js
made hover() and blur() methods private and added check for null value in _hover()
@Peinkillar

This comment has been minimized.

Show comment Hide comment
@Peinkillar

Peinkillar Sep 5, 2012

The methods can be private of course but why aren't your change and select method? I added the check for null in _hover() too. Do you think it is ok now?

The methods can be private of course but why aren't your change and select method? I added the check for null in _hover() too. Do you think it is ok now?

@fnagel

This comment has been minimized.

Show comment Hide comment
@fnagel

fnagel Sep 5, 2012

Owner

You are aware I only maintain this plugin as its original developed by filament group? Since then a bunch of people contributed and the code is everything but ot straight forward and clean. I'm just a guy who puts lot of his free time in this to keep this working until the official version is released, see #140

Change is public because its triggers a change event on the original select. I guess.
Select is public because... good question. Should be private I guess. TODO

Perhaps we should add your callbacks inline, without extra functions as they are really small. What do you think?

Please update your branch!

Owner

fnagel commented Sep 5, 2012

You are aware I only maintain this plugin as its original developed by filament group? Since then a bunch of people contributed and the code is everything but ot straight forward and clean. I'm just a guy who puts lot of his free time in this to keep this working until the official version is released, see #140

Change is public because its triggers a change event on the original select. I guess.
Select is public because... good question. Should be private I guess. TODO

Perhaps we should add your callbacks inline, without extra functions as they are really small. What do you think?

Please update your branch!

made blur and hover methods inline
and removed unneeded check for null because the element at the index
should actually be never null. otherwise it wouldn't be triggered
@Peinkillar

This comment has been minimized.

Show comment Hide comment
@Peinkillar

Peinkillar Sep 7, 2012

Owner

Sorry about this commit with 882 additions and 892 deletions ... I am not familiar with github so far. Do I have to update my version first and how?

Owner

Peinkillar commented on 6b87f0a Sep 7, 2012

Sorry about this commit with 882 additions and 892 deletions ... I am not familiar with github so far. Do I have to update my version first and how?

@fnagel

This comment has been minimized.

Show comment Hide comment
@fnagel

fnagel Sep 21, 2012

Owner

You need to merge your branch with my selectmenu branch, than pushing your branch to GitHub again.

Owner

fnagel commented Sep 21, 2012

You need to merge your branch with my selectmenu branch, than pushing your branch to GitHub again.

@fnagel

This comment has been minimized.

Show comment Hide comment
@fnagel

fnagel Sep 21, 2012

Owner

Sorry for the late response. Vacation time :-)

Owner

fnagel commented Sep 21, 2012

Sorry for the late response. Vacation time :-)

@Peinkillar

This comment has been minimized.

Show comment Hide comment
@Peinkillar

Peinkillar Sep 23, 2012

I'm on vacation as well ;) I will do it when I am back.

I'm on vacation as well ;) I will do it when I am back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment