Skip to content

Conversation

@diogo-pessoa
Copy link
Contributor

Description

Remove pop-up leave site upon user action for back button or refresh triggering the unload event.

Tested with firefox, Chrome and safari from local Refocus, pop-up behaviour was suppressed after change.

I've also tested the TimelineBot to confirm the post command is still triggered and broadcasts the user left room Event that feeds the Bot with the users exiting room.

User Joined - admin@refocus.admin
10:51 UTC | 07/08/2020

admin@refocus.admin has joined room.
User Left - admin@refocus.admin
10:51 UTC | 07/08/2020

admin@refocus.admin has left room.
User Joined - admin@refocus.admin
10:51 UTC | 07/08/2020

admin@refocus.admin has joined room.
User Left - admin@refocus.admin
10:51 UTC | 07/08/2020

admin@refocus.admin has left room.

Line 679:
Setting undefined as the return for function set on beforeUnload Event according to developer.mozilla some browsers might ignore the null and prompt the user with the leave site? confirmation pop-up.

When this event returns (or sets the returnValue property to) a value other than null or undefined, the user will be prompted to confirm the page unload. In older browsers, the return value of the event is displayed in this dialog. Starting with Firefox 44, Chrome 51, Opera 38, and Safari 9.1, a generic string not under the control of the webpage will be shown instead of the returned string
...Internet Explorer does not respect the null return value and will display this to users as "null" text. You have to use undefined to skip the prompt.

Line 1074:

Typically, it is better to use window.addEventListener() and the beforeunload event, instead of onbeforeunload.

Reference:

https://developer.mozilla.org/en-US/docs/Web/API/WindowEventHandlers/onbeforeunload

@coveralls
Copy link

coveralls commented Jul 8, 2020

Coverage Status

Coverage remained the same at 88.751% when pulling 4cfe969 on removingLeavePopup into 7b10406 on master.

@cassiodias cassiodias requested a review from LongweiDeng July 9, 2020 09:10
Copy link
Contributor

@BrandonMurnane BrandonMurnane left a comment

Choose a reason for hiding this comment

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

nice change

@diogo-pessoa diogo-pessoa merged commit 0a8b1c6 into master Jul 9, 2020
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.

7 participants