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

romoAjax: fix bug with invokes with data #138

Merged
merged 1 commit into from Oct 5, 2017

Conversation

Projects
None yet
2 participants
@kellyredding
Member

kellyredding commented Oct 4, 2017

This adds a second event handler, onTriggerInvoke that is now
used exculsively with the 'romoAjax:triggerInvoke' event. This
allows having separate event processing for triggered invokes
vs auto invokes (from a click for example). Auto invokes never
expect/take data where trigger invokes do. This fixes a bug
where you use an event that passes unrelated data as an auto
invoke. Without this, the unrelated data would be interpreted
as invoke data which is not intended.

This also adds return false directives to the picker's romo
ajax event handlers. Since romo ajax components are prone to
being nested, it is a good practice to stop propagation on any
romo ajax events. This handles that for pickers.

@jcredding ready for review.

romoAjax: fix bug with invokes with data
This adds a second event handler, `onTriggerInvoke` that is now
used exculsively with the 'romoAjax:triggerInvoke' event.  This
allows having separate event processing for triggered invokes
vs auto invokes (from a click for example).  Auto invokes never
expect/take data where trigger invokes do.  This fixes a bug
where you use an event that passes unrelated data as an auto
invoke.  Without this, the unrelated data would be interpreted
as invoke data which is not intended.

This also adds `return false` directives to the picker's romo
ajax event handlers.  Since romo ajax components are prone to
being nested, it is a good practice to stop propagation on any
romo ajax events.  This handles that for pickers.
@kellyredding

This comment has been minimized.

Show comment
Hide comment
@kellyredding

kellyredding Oct 5, 2017

Member

@jcredding I'm going to go ahead and merge this, FYI. Please do look at it when you get a chance but I need to move on with this change. Thanks.

Member

kellyredding commented Oct 5, 2017

@jcredding I'm going to go ahead and merge this, FYI. Please do look at it when you get a chance but I need to move on with this change. Thanks.

@jcredding

This comment has been minimized.

Show comment
Hide comment
@jcredding

jcredding Oct 5, 2017

Member

@kellyredding - I'm good with this 💥 Sorry meant to look at it this morning.

Member

jcredding commented Oct 5, 2017

@kellyredding - I'm good with this 💥 Sorry meant to look at it this morning.

@kellyredding

This comment has been minimized.

Show comment
Hide comment
@kellyredding

kellyredding Oct 5, 2017

Member

@jcredding no worries, thanks man.

Member

kellyredding commented Oct 5, 2017

@jcredding no worries, thanks man.

@kellyredding kellyredding merged commit d4e968c into master Oct 5, 2017

@kellyredding kellyredding deleted the kr-picker-ajax-cleanup branch Oct 5, 2017

kellyredding added a commit that referenced this pull request Oct 5, 2017

version to 0.19.10
* (hotfix) better test setup/teardown for Dassets tests 7b52881
* romoAjax: fix bug with invokes with data #138
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment