Execute pys-on* events when triggered, not at load #686

JeffersGlass · 2022-08-12T18:25:27Z

Pys-on* attributes should behave the same as Javascript on attributes (onClick, onKeyDown, etc.), which executed at event-trigger-time and not passed a reference to a function. (See, for example, freecodecamp for examples in Javascript.)

Fixes #685 - see example code in that issue for illustration.

marimeireles · 2022-08-25T22:08:19Z

Will check this more closely tomorrow morning.
But makes a lot of sense to me, thanks for the explanation in depth on your issue!

JeffersGlass · 2022-08-25T23:06:47Z

Well, by the powers of me and VSCode visual merge combined, I have screwed up merging the commit to fix the examples. I'll have to fix that tomorrow. Sorry!

marimeireles · 2022-08-26T13:11:23Z

Merging is soo 😅 visual or not, is so often confusing
No problem!

marimeireles · 2022-08-29T22:13:14Z

I think what's happening here is that Madhur changed a couple of things related to how we run Pyodide and now the tests are expecting this behavior.
This is the relevant PR: https://github.com/pyscript/pyscript/pull/698/files
At least that's how it's failing locally to me, it's different from the CI... Might be a problem on my environment.
LMK if you'd like input to debug this, thanks Jeff.

JeffersGlass · 2022-08-31T20:49:49Z

Thanks for pointing that out - makes sense to me. I see 2 failing integration tests, one is now fix, the other is odd:

test_repl2: This was caused by my implementation not awating runtime.run(...). That has been fixed by the most recent commit.
test_toga_freedom: Looks like this is loading wheels for a custom build of toga built on this unmerged PR, which uses pys-onclick=function Not sure what the right thing to do is here... it's not really demoing something you can actually do in toga currrently, unless you also build against this unmerged PR from two months ago. I can modify the source within those wheels to make this work, if that's what's wanted though.

I also had test_panel_deckgl failing initially, but just because it was taking more than 30 seconds to initialize on my particular machine.

Are there other tests that are failing in your environment?

(Still to-do is @fpliger's note on the issue (#685) to leave the pys-on* events with their current behavior, but mark them and their behavior as deprecated.)

Mimicing the behavior of Javascripts 'onLoad' event, we should not be executing the use code at page-load time, only when the event is triggered.

for more information, see https://pre-commit.ci

marimeireles · 2022-09-12T15:24:17Z

test_repl2: This was caused by my implementation not awating runtime.run(...). That has been fixed by the most recent commit.

I've just rebased everything on main to test it on the latest version of the code and it doesn't seem to be working.
Any guesses here?

test_toga_freedom: Looks like this is loading wheels for a custom build of toga beeware/toga#1475, which uses pys-onclick=function Not sure what the right thing to do is here... it's not really demoing something you can actually do in toga currently, unless you also build against this unmerged PR from two months ago. I can modify the source within those wheels to make this work, if that's what's wanted though.

Okay, maybe this is another good candidate for us to remove from pyscript -> collective.
@fpliger opinions here?

fpliger · 2022-09-12T19:41:51Z

+1 on moving it to the collective. The examples is too out of pyscript-core that makes sense for the core devs not maintaining it directly. @freakboy3742 any thoughts?

test_repl2 seems to be working for me locally. For test_todo failing I think there's something going on there and needs some investigation... I've bumped the timeout limit to 120 locally and it's still failing... :/

ayirpconda · 2022-09-13T00:31:37Z

+1 on moving it to the collective. The examples is too out of pyscript-core that makes sense for the core devs not maintaining it directly. @freakboy3742 any thoughts?

test_repl2 seems to be working for me locally. For test_todo failing I think there's something going on there and needs some investigation... I've bumped the timeout limit to 120 locally and it's still failing... :/

+1 on moving it to the collective. The examples is too out of pyscript-core that makes sense for the core devs not maintaining it directly. @freakboy3742 any thoughts?

Yes +1 on PyScript Collective as home for the neat examples

freakboy3742 · 2022-09-14T00:18:19Z

Moving the examples out of the core repo makes sense to me.

FWIW: I'm hoping to spend some time working on the Toga web backend (and adding a Briefcase web backend) in early October, so I should have some updates for the Toga demos in the near future.

JeffersGlass · 2022-09-14T12:49:40Z

Sounds good all - I won't worry about the Toga Example failing for the moment, if that's being moved into the collective.

I've just rebased everything on main to test it on the latest version of the code and it [test_repl2] seem to be working.
Any guesses here?

test_repl2 seems to be working for me locally. For test_todo failing I think there's something going on there and needs some investigation... I've bumped the timeout limit to 120 locally and it's still failing... :/

Thanks @marimeireles and @fpliger - I'll rebase locally have a look at these and see if I can figure out what's happening. Interesting that there are different results for each of us. I've mostly been testing on a (slow-ish) Linux VM. Maybe I'll try setting up to test on a faster Windows box, to see if there's a potential timeout or race condition or something going on here...

Edit: Ah, nevermind about testing on Windows - support.py uses the Py library and specifically path.mksymlinkto(), which appears to only work for POSIX paths. Maybe we consider moving to Pathlib or os.path in the future?

…tests (pyscript#747) * Create custom elements when the runtime finishes loading * Remove xfails and fix repl integration test * Fix commented ignore * Address Antonio's comments * Fix bad rebase * Make ure to wait for repl to be in attached state before asserting content * Move createCustomeElement up so it runs before we close the loader, xfail flaky d3 test * Fix xfail

updates: - [github.com/pre-commit/mirrors-eslint: v8.23.0 → v8.23.1](pre-commit/mirrors-eslint@v8.23.0...v8.23.1) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

…ript#764)

JeffersGlass · 2022-09-14T21:46:14Z

test_repl2 now works. Probably due to some improvements made by #747 yesterday.

test_todo is now working, after fixing a typo of mine.

The new deprecation of pys-on* events actually means test_toga_freedom passes. I've marked it as xfail with a note about moving it to the collective, so that future changes that break that example aren't a roadblock.

fpliger

@JeffersGlass Thank you so much for the contribution and for the patience to move this through! This is a great PR! It also anticipates some of the optimizations we need to do to move things when the are ready (like deferring custom elements creations). We'll need to keep you a PyScript shirt the next time we have them! 😄

fpliger · 2022-09-15T01:33:24Z

Merging since the failing tests is due to the doc changes and not related to the code changes....

JeffersGlass · 2022-09-15T12:56:59Z

Thanks @fpliger! More than happy to work through this, and honestly got to learn a lot more about the integration-test architecture along the way, which was good fun and I think will be very useful.

marimeireles · 2022-09-15T14:11:58Z

This is such great PR, srsly.
Just used it today! :D
Thanks @JeffersGlass!

marimeireles mentioned this pull request Aug 26, 2022

py-on* event handler code is being executed at load time #685

Closed

3 tasks

marimeireles force-pushed the fix-pys-event-execution branch from 439c0b2 to 2efad34 Compare August 29, 2022 18:17

marimeireles added the waiting on feedback Issue or PR waiting on feedback from core team label Sep 12, 2022

JeffersGlass and others added 6 commits September 12, 2022 16:38

Execute pys-on* events when triggered, not at load

c321727

Mimicing the behavior of Javascripts 'onLoad' event, we should not be executing the use code at page-load time, only when the event is triggered.

Update examples to new syntax

d0bd2de

Fix merge issue

8b594ed

Await running event handler code

990d51e

Restore pys-on* events with original behavior, deprecation warning

f9a562c

[pre-commit.ci] auto fixes from pre-commit.com hooks

02a7178

for more information, see https://pre-commit.ci

marimeireles force-pushed the fix-pys-event-execution branch from f4a7084 to 02a7178 Compare September 12, 2022 14:38

marimeireles added waiting on reporter Further information is requested from the reported and removed waiting on feedback Issue or PR waiting on feedback from core team labels Sep 12, 2022

marimeireles mentioned this pull request Sep 14, 2022

Move Examples PRs from main repo pyscript/pyscript-collective#6

Open

JeffersGlass and others added 7 commits September 14, 2022 11:40

xfail toga example

e6be695

Add missing { (typo)

361f91d

Adjust callback chandling to make linter happy

c536f24

Change alpha to latest (pyscript#760)

3438913

[pre-commit.ci] pre-commit autoupdate (pyscript#762)

dd6f29f

updates: - [github.com/pre-commit/mirrors-eslint: v8.23.0 → v8.23.1](pre-commit/mirrors-eslint@v8.23.0...v8.23.1) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

change documentation to point to latest instead of frozen alpha (pysc…

7eb1436

…ript#764)

JeffersGlass added 2 commits September 14, 2022 16:26

Toga example is xpass

98fba5c

Correct 'xpass' to 'xfail' mark

2a7cfcc

fpliger approved these changes Sep 15, 2022

View reviewed changes

fpliger merged commit 0b014ee into pyscript:main Sep 15, 2022

JeffersGlass deleted the fix-pys-event-execution branch September 15, 2022 12:54

JeffersGlass mentioned this pull request Sep 28, 2022

Loading and void script.evaluate() #796

Merged

JeffersGlass mentioned this pull request Oct 6, 2022

Behaviour of py-*: how to pass event #835

Closed

3 tasks

JeffersGlass mentioned this pull request Jul 5, 2023

[next] Allow direct events handling via attributes #1574

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Execute pys-on* events when triggered, not at load #686

Execute pys-on* events when triggered, not at load #686

JeffersGlass commented Aug 12, 2022 •

edited

Loading

marimeireles commented Aug 25, 2022

JeffersGlass commented Aug 25, 2022

marimeireles commented Aug 26, 2022

marimeireles commented Aug 29, 2022 •

edited

Loading

JeffersGlass commented Aug 31, 2022

marimeireles commented Sep 12, 2022

fpliger commented Sep 12, 2022

ayirpconda commented Sep 13, 2022

freakboy3742 commented Sep 14, 2022

JeffersGlass commented Sep 14, 2022 •

edited

Loading

JeffersGlass commented Sep 14, 2022

fpliger left a comment

fpliger commented Sep 15, 2022

JeffersGlass commented Sep 15, 2022

marimeireles commented Sep 15, 2022

Execute pys-on* events when triggered, not at load #686

Execute pys-on* events when triggered, not at load #686

Conversation

JeffersGlass commented Aug 12, 2022 • edited Loading

marimeireles commented Aug 25, 2022

JeffersGlass commented Aug 25, 2022

marimeireles commented Aug 26, 2022

marimeireles commented Aug 29, 2022 • edited Loading

JeffersGlass commented Aug 31, 2022

marimeireles commented Sep 12, 2022

fpliger commented Sep 12, 2022

ayirpconda commented Sep 13, 2022

freakboy3742 commented Sep 14, 2022

JeffersGlass commented Sep 14, 2022 • edited Loading

JeffersGlass commented Sep 14, 2022

fpliger left a comment

Choose a reason for hiding this comment

fpliger commented Sep 15, 2022

JeffersGlass commented Sep 15, 2022

marimeireles commented Sep 15, 2022

JeffersGlass commented Aug 12, 2022 •

edited

Loading

marimeireles commented Aug 29, 2022 •

edited

Loading

JeffersGlass commented Sep 14, 2022 •

edited

Loading