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

Show alert when user leaves #886

Merged

Conversation

iAmAshuSahoo
Copy link
Contributor

@iAmAshuSahoo iAmAshuSahoo commented Apr 28, 2024

Date: 28 Apr 2024

Developer Name: Ashutosh Sahoo


Issue Ticket Number:-

Alert should be shown

Description:

  1. Alert should be shown when the stream is running and the user tries to navigate to a different page.
  2. Alert needs to be shown when the user is trying to close the window/tab where live is running.

Is Under Feature Flag

  • Yes
  • No

Database changes

  • Yes
  • No

Breaking changes (If your feature is breaking/missing something please mention pending tickets)

  • Yes
  • No

Is Development Tested?

  • Yes
  • No

Tested in staging?

  • Yes
  • No

Add relevant Screenshot below ( e.g test coverage etc. )

Test on firefox
image

Test on Chrome
image

Test Coverage
image

Build
image

@iAmAshuSahoo
Copy link
Contributor Author

iAmAshuSahoo commented Apr 28, 2024

Ways taken for solving that DIDN'T worked

  1. Adding window.addeventlistener in constructor of Controller, Component.
constructor() {
    super(...arguments);
    window.addEventListener('beforeunload', this.confirmLeave);
  }
willDestroy() {
    window.removeEventListener('beforeunload', this.confirmLeave);
    super.willDestroy(...arguments);
  }
@action
  confirmLeave(event) {
    event.preventDefault();
    event.returnValue = this.confirmationMessage;
    return this.confirmationMessage;
  }

Got error - TypeError: window.addEventListener is not a function

  1. Added the same logic to Route as well with init and didInsertElement.
  2. Using confirmation as a service and invoking this in hbs.
  3. <div id="live" data-test-live onbeforeunload="return 'Are you sure you want to leave? Your changes may not be saved.';">
  4. Tried adding the addon ember-onbeforeunload that uses the concepts of mixin. This is deprecated
  5. Custom message
    https://stackoverflow.com/questions/38879742/is-it-possible-to-display-a-custom-message-in-the-beforeunload-popup

@palakgupta2712
Copy link
Contributor

@iAmAshuSahoo

  1. Kindly update the PR description.
  2. Please check CI is failing.

Copy link

@mehra-sourav mehra-sourav left a comment

Choose a reason for hiding this comment

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

Please add tests

@tejaskh3
Copy link
Contributor

tejaskh3 commented May 1, 2024

kindly add the date and developer name to the PR.
image

Copy link
Contributor

@tejaskh3 tejaskh3 left a comment

Choose a reason for hiding this comment

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

image
Have you worked on this task before, then please add reference to those previous PRs and add staging proof too. if not then please mark this as "NO"

@iAmAshuSahoo
Copy link
Contributor Author

@iAmAshuSahoo

1. Kindly update the PR description.

2. Please check CI is failing.
  1. Updated description
  2. CI is checked.
  3. Added tests.
  4. updated PR information

app/templates/live.hbs Outdated Show resolved Hide resolved
app/services/live.js Outdated Show resolved Hide resolved
tests/acceptance/live-test.js Outdated Show resolved Hide resolved
Copy link
Member

@satyam73 satyam73 left a comment

Choose a reason for hiding this comment

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

LGTM

@iamitprakash iamitprakash merged commit 1fc9640 into Real-Dev-Squad:develop May 19, 2024
2 checks passed
@iAmAshuSahoo iAmAshuSahoo mentioned this pull request May 19, 2024
10 tasks
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.

8 participants