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

Raise Exception when awaiting JavaScript on auto-index page #2983

Merged
merged 7 commits into from
May 11, 2024

Conversation

falkoschindler
Copy link
Contributor

@falkoschindler falkoschindler commented Apr 29, 2024

This PR tries to solve #2951 by preventing the user from awaiting any JavaScript call on the auto-index page.

As it turned out, many existing pytests had to be updated, since they didn't use page functions but awaited JavaScript.

There's a last problem with ui.notification, which currently can't be used on auto-index pages at all. It continuously checks if it is still contained in the DOM or if it should be deleted. With this PR in place, this check raises a RuntimeError.

Apart from ui.notification, the clipboard.read() function isn't usable on the auto-index page. Conceptually this is correct. But we will need to update corresponding demos which don't use page functions at the moment.

  • update clipboard demos
  • make ui.notification usable on auto-index page --> trigger event when notification disappears on one client and delete it for all others

@falkoschindler falkoschindler added this to the 1.4.24 milestone Apr 29, 2024
@falkoschindler falkoschindler linked an issue Apr 29, 2024 that may be closed by this pull request
@falkoschindler falkoschindler added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Apr 29, 2024
@falkoschindler falkoschindler self-assigned this May 2, 2024
@falkoschindler falkoschindler modified the milestones: 1.4.24, 1.4.25 May 6, 2024
@falkoschindler
Copy link
Contributor Author

As a side effect of fixing the deletion of notifications in 5235446, we now can register "dismiss" events using the new on_dismiss parameter and pass JavaScript handlers for action buttons like this:

ui.notification('Hi!', close_button=True, on_dismiss=lambda: ui.notify('Dismissed'))
ui.notification('Ho!', actions=[{'label': 'Alert', ':handler': '() => alert("Alert!")'}])

@falkoschindler falkoschindler marked this pull request as ready for review May 10, 2024 16:39
@falkoschindler falkoschindler requested a review from rodja May 10, 2024 16:39
@falkoschindler
Copy link
Contributor Author

There seems to be a problem with codemirror, but I'm not sure if it's coming from this PR or the main branch. I'll look into it later.

@falkoschindler
Copy link
Contributor Author

I think I fixed the problem in 154cb73.

@rodja rodja merged commit 5c444ff into main May 11, 2024
7 checks passed
@rodja rodja deleted the await_javascript branch May 11, 2024 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When executing the run_method , a KeyError error occurs.
2 participants