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: separate creation of browser notifications and sounds from Vue rendering #1947

Merged
merged 3 commits into from
Jun 13, 2024

Conversation

Antreesy
Copy link
Collaborator

@Antreesy Antreesy commented Jun 11, 2024

☑️ Resolves

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

No visual changes

🚧 Tasks

  • check Web Worker - doesn't have access to window, OC, and other global variables, requires webpack change
  • check active/inactive state
  • check frozen/ discarded states for suspended tabs

🏁 Checklist

  • 🌏 Tested with Chrome, Firefox and Safari or should not be risky to browser differences
  • 🖥️ Tested with Desktop client or should not be risky for it

Copy link
Member

@AndyScherzinger AndyScherzinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐘

…omponent

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy force-pushed the fix/1946/detach-native-notifications branch from a6cd273 to 2dce190 Compare June 13, 2024 09:12
@Antreesy
Copy link
Collaborator Author

/compile /

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still plays sound in normal tabs and inactive tabs with Firefox.
Couldn't test the chrome suspension thou

Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good code wise.

Testing results:

State Before After Notes
hidden ✅ shows notification ✅ shows notification -
frozen ? ? hard to test without waiting for a long time, chrome://discards has no "freeze" option
discarded ❌ no notification ❌ no notification only solvable via worker

@Antreesy Antreesy merged commit cfa56a8 into master Jun 13, 2024
37 checks passed
@Antreesy Antreesy deleted the fix/1946/detach-native-notifications branch June 13, 2024 10:07
@Antreesy
Copy link
Collaborator Author

/backport d3ad4b1 2dce190 to stable29

@Antreesy
Copy link
Collaborator Author

/backport d3ad4b1 2dce190 to stable28

@Antreesy
Copy link
Collaborator Author

/backport d3ad4b1 2dce190 to stable27

@nickvergessen
Copy link
Member

/backport d3ad4b1 2dce190 to stable27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants