Skip to content

refactor: enhance event system rust apis#7996

Merged
lucasfernog merged 16 commits intodevfrom
refactor/event-system-apis
Oct 23, 2023
Merged

refactor: enhance event system rust apis#7996
lucasfernog merged 16 commits intodevfrom
refactor/event-system-apis

Conversation

@amrbashir
Copy link
Copy Markdown
Member

@amrbashir amrbashir commented Oct 10, 2023 •
edited
Loading

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

The goal of this PR is to remove the trigger term from the APIs which I don't see very often and would prefer to ditch and stick to listen and emit only.

Changed emit to emit events to JS and Rust side which should be the default IMO but should we provide another set of APIs to target either JS or Rust?

@amrbashir amrbashir requested a review from a team as a code owner October 10, 2023 16:56
@amrbashir amrbashir marked this pull request as draft October 10, 2023 17:05
@amrbashir
Copy link
Copy Markdown
Member Author

amrbashir commented Oct 11, 2023 •
edited
Loading

So I keep going back and forth between two decisions about one final piece of this PR which is Manager::listen_global and Manager::once_global:

  1. Keep them as is, to know that it is global
  2. Renamed them to Manager::listen and Manager::once, and when used from App/AppHandle it will be global and when used from Window it will be specific to this window.

Also since I have changed emit to emit events to JS and Rust side and not only JS side, which should be the default IMO but should we provide another set of APIs to target either JS or Rust?

@lucasfernog
Copy link
Copy Markdown
Member

lucasfernog commented Oct 16, 2023

I think @chippers might help you decide since he designed most of these APIs :)

@amrbashir amrbashir marked this pull request as ready for review October 18, 2023 00:42
@amrbashir
Copy link
Copy Markdown
Member Author

amrbashir commented Oct 18, 2023

I think this PR is ready to go, I have decided to keep Window::listen to be specific to window and Manager::listen_global to be global. I am not the biggest fan of having _global prefix but good enough for now.

@lucasfernog lucasfernog merged commit 93c8a77 into dev Oct 23, 2023
@lucasfernog lucasfernog deleted the refactor/event-system-apis branch October 23, 2023 18:10
olivierlemasle added a commit to olivierlemasle/tauri that referenced this pull request Oct 25, 2023
@olivierlemasle
fix: fix example/api build on mobile
b6ee665 
Since tauri-apps#7996, `Window::emit` has been moved to `Manager::emit`,
but `tauri::Manager` is imported into scope only for
`cfg(desktop)`.
lucasfernog added a commit that referenced this pull request Oct 30, 2023
@lucasfernog
fix(core): global events regression from #7996 closes #8146
d9973fa
lucasfernog added a commit that referenced this pull request Nov 7, 2023
@lucasfernog
fix(core): global events regression from #7996 closes #8146 (#8147)
b5f40ae
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lucasfernog lucasfernog lucasfernog approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Roadmap
Status: 🔎 In audit

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@amrbashir @lucasfernog