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

Convert Dirty from Coffeescript to Javascript #2510

Merged

Conversation

sascha-karnatz
Copy link
Contributor

What is this pull request for?

Convert dirty.js.coffee to dirty.js and move the file into app/javascript.

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

@sascha-karnatz sascha-karnatz requested a review from tvdeyen July 3, 2023 08:01
return $(element).hasClass("dirty")
}

function checkPageDirtyness(element) {
Copy link

Choose a reason for hiding this comment

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

Function checkPageDirtyness has 27 lines of code (exceeds 25 allowed). Consider refactoring.

@sascha-karnatz sascha-karnatz force-pushed the move-dirty.js-into-app-javascript branch from 1154ed2 to bf3f486 Compare July 4, 2023 06:04
@tvdeyen tvdeyen added this to the 7.1 milestone Jul 4, 2023
@sascha-karnatz sascha-karnatz force-pushed the move-dirty.js-into-app-javascript branch 2 times, most recently from 6129e4c to 9bd0277 Compare July 6, 2023 06:53
@tvdeyen
Copy link
Member

tvdeyen commented Jul 31, 2023

@sascha-karnatz needs a rebase now that #2509 has been merged

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

This does not trigger the unload event on page refresh anymore. Try reloading the page if an element is in dirty state in latest main. It will trigger an confirm dialog. In this branch it does not show this confirm dialog anymore.

@sascha-karnatz sascha-karnatz force-pushed the move-dirty.js-into-app-javascript branch 2 times, most recently from 9d7fea2 to 50d3e7d Compare July 31, 2023 20:29
@sascha-karnatz
Copy link
Contributor Author

This does not trigger the unload event on page refresh anymore. Try reloading the page if an element is in dirty state in latest main. It will trigger an confirm dialog. In this branch it does not show this confirm dialog anymore.

Done. It was a missing function.

Also move the file to app/javascript folder.
@sascha-karnatz sascha-karnatz force-pushed the move-dirty.js-into-app-javascript branch from 50d3e7d to 04740e0 Compare August 2, 2023 06:19
@tvdeyen tvdeyen merged commit 8596f74 into AlchemyCMS:main Aug 2, 2023
@sascha-karnatz sascha-karnatz deleted the move-dirty.js-into-app-javascript branch August 2, 2023 12:21
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