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

DT-340: Warn the user if they are leaving a story edit page with unsaved changes #334

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

torresga
Copy link
Contributor

@torresga torresga commented Nov 7, 2024

Jira Ticket

https://ombulabs.atlassian.net/browse/DT-340

Motivation / Context

Currently if you leave the current page, you can lose information that was not saved. This feature warns you if you are leaving without saving information.

QA / Testing Instructions

  1. Go to the Edit Story page.
  2. Edit the story.
  3. Try to navigate off the page by reloading, pressing the back button in the browser, clicking the logo or the Back button on the page.
  4. Ensure that you get a popup asking you if are sure you want to reload/go back.

Screenshots:

Screenshot 2024-11-07 at 1 53 28 PM

I will abide by the code of conduct.

…e form existed. Also created a function to add and remove the beforeunload event listener
@torresga torresga marked this pull request as ready for review November 7, 2024 20:30
Copy link
Member

@JuanVqz JuanVqz left a comment

Choose a reason for hiding this comment

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

My suggestions are not a blocker.

Comment on lines +53 to +60
if (!confirmLeave) {
// Prevent navigation if the user chooses not to leave
event.preventDefault();
} else {
// Optionally, reset isDirty if leaving
isDirty = false;
addBeforeUnloadEventListener(isDirty)
}
Copy link
Member

Choose a reason for hiding this comment

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

I find it easier to read confirmLeave compared to !confirmLeave, as it always takes me a moment to grasp the condition.

Suggested change
if (!confirmLeave) {
// Prevent navigation if the user chooses not to leave
event.preventDefault();
} else {
// Optionally, reset isDirty if leaving
isDirty = false;
addBeforeUnloadEventListener(isDirty)
}
if (confirmLeave) {
// Optionally, reset isDirty if leaving
isDirty = false;
addBeforeUnloadEventListener(isDirty)
} else {
// Prevent navigation if the user chooses not to leave
event.preventDefault();
}

}
}

function warnUserifUnsavedEdits(event) {
Copy link
Member

Choose a reason for hiding this comment

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

A bit picky, you might missed the I should go in uppercase.

Suggested change
function warnUserifUnsavedEdits(event) {
function warnUserIfUnsavedEdits(event) {

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.

2 participants