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

Fix flushing commands correctly #28

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix flushing commands correctly #28

wants to merge 1 commit into from

Conversation

tho-graf
Copy link

  • before:
    -- only dispatch flushed the commands and not dispatchCapturedEvent
    -- the commands were flushed without checking if it was really the
    beginning or the end of an event.

  • now the dispatch and the dispatchCapturedEvent is wrapped into gwt's
    $entry method (Impl::entry), which handles all the stuff

@tho-graf
Copy link
Author

@laaglu can you have a look at this? Thanks!

@laaglu
Copy link
Owner

laaglu commented Dec 21, 2018

Hi. Thanks for the PR. I have not touched GWT code for a long time and but from what I read in the doc, it seems wrapping native to Java code in $entry() like you propose makes sense (http://www.gwtproject.org/doc/latest/DevGuideCodingBasicsJSNI.html).

I would like to test the patch on my own apps to make sure it has no unforseen side-effects. If this works, I will merge your PR which will be in the next version of lib-gwt-svg, whenever that is (lib-gwt-svg is in maintenance mode, I only release it when there is some significant change in the GWT ecosystem. I suppose all these new versions of Java will sooner or later cause a new version of GWT come out).

- before:
-- only dispatch flushed the commands and not dispatchCapturedEvent
-- the commands were flushed without checking if it was really the
beginning or the end of an event.

- now the dispatch and the dispatchCapturedEvent is wrapped into gwt's
$entry method (Impl::entry), which handles all the stuff
@tho-graf
Copy link
Author

tho-graf commented Jan 11, 2019

Hi, thanks for your answer.

I updated the PR with one bugfix.

I had $entry(dispatchCapturedEvent).call(($wnd.__helperImpl, evt, evt.currentTarget));

The inner parentheses around $wnd.__helperImpl, evt, evt.currentTarget were too much. In JS, this means all values/functions get evaluated and the last values gets returned.

We already use this version in production and encountered no other bugs.

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