Skip to content

Conversation

deefour
Copy link
Contributor

@deefour deefour commented Feb 7, 2012

When using a custom render() method containing more than 1 level of
nesting, the onClick event is not triggered when not clicking
directly on the root .text-suggestion element.

Clicking anywhere in the green box will not trigger the onClick.
Clicking in the red box (a small sliver at the bottom of the
.text-suggestion)
does trigger the onClick.

This changeset causes the onMouseOver and onClick events to look at
the target's ancestor's to see if one of them is a .text-suggestion,
swapping the target to that DOM element if found.

When using a custom `render()` method containing more than 1 level of
nesting, the `onClick` event is not triggered when not clicking
*directly* on the root `.text-suggestion` element.

 - http://goo.gl/MpY8f

Clicking anywhere in the green box will not trigger the `onClick`.
Clicking in the red box *(a small sliver at the bottom of the
`.text-suggestion`)* **does** trigger the `onClick`.

This changeset causes the `onMouseOver` and `onClick` events to look at
the `target`'s ancestor's to see if one of them is a `.text-suggestion`,
swapping the `target` to that DOM element if found.
@mreinstein
Copy link
Contributor

What's the status on this? I'm trying to use textext in a client project, and I'm getting a lot of grief that the suggestions arent clickable. This seems to me like very obvious behavior. I mean if there's a suggestion box, it stands to reason many people will try to click the list item. Is this patch being held back for some reason?

@deefour
Copy link
Contributor Author

deefour commented Feb 27, 2012

@mreinstein I'm not sure why it hasn't been merged yet, but you can always fork the project yourself and merge this pull request, or just use the code from my fork in the meantime.

@mreinstein
Copy link
Contributor

@deefour, I guess I was just hoping for clarification on why a simple but really important fix hasnt been merged in the last 3 weeks. Curious if Alex had an issue with the way the fix was done, or if there was another reason. I'll manually apply the patch for now, and thanks for the heads-up!

@mreinstein
Copy link
Contributor

+1

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