Warn for changes just in markdown editor

This is the reason why this feature was implemented in the first
place: it's easy to open the editor, make some changes, close it, and
continue without realizing the changes have not been saved.

In the rest of the forms, this functionality is quite lacking. For
starters, some forms warn if there are unsaved changes, while some forms
don't, which is highly inconsistent and disorients users.

Furthermore, we were having problems with this feature after upgrading
Turbolinks, particularly in forms using CKEditor. In these cases, a lot
of hacking needs to be done in order to make this feature work properly,
since CKEditor adds some formatting automatically, and if this is done
after the form is serialized, we'll get some unexpected behavior. On the
other hand, comparing the value of a textarea against its `defaultValue`
property will work on every edge case, including using the browser's
back button or reloading the page.

Finally, users are used to the way web forms work, and aren't used to be
asked for confirmation when they change their mind and decide to leave
the page without saving the changes. Asking them for confirmation will
be annoying in most cases. Besides that, if they accidentally leave the
page, they can use the browser's back button and they'll recover the
unsaved changes.

It's true this won't happen it they accidentally close the browser's
window, but our WatchFormChanges functionality didn't work in this case
either. Using the "beforeunload" event adds more problems than it
solves, since it doesn't support custom messages (or, to be more
precise, modern browsers ignore custom messages), and it doesn't get
along with turbolinks.

Co-Authored-By: Senén Rodero Rodríguez <senenrodero@gmail.com>

app/assets/javascripts/application.js

@@ -87,7 +87,7 @@
 //= require legislation
 //= require legislation_allegations
 //= require legislation_annotatable
-//= require watch_form_changes
+//= require legislation_draft_versions
 //= require followable
 //= require flaggable
 //= require documentable
@@ -145,7 +145,6 @@ var initialize_modules = function() {
   if ($(".legislation-annotatable").length) {
     App.LegislationAnnotatable.initialize();
   }
-  App.WatchFormChanges.initialize();
   App.TreeNavigator.initialize();
   App.Documentable.initialize();
   App.Imageable.initialize();

app/assets/javascripts/legislation_draft_versions.js

@@ -0,0 +1,22 @@
+(function() {
+  "use strict";
+  App.LegislationDraftVersions = {
+    msg: function() {
+      return $("[data-markdown-changes-message]").data("markdown-changes-message");
+    },
+    hasChanged: function() {
+      return $(".markdown-editor textarea").is(function() {
+        return this.value !== this.defaultValue;
+      });
+    },
+    checkChanges: function() {
+      if (App.LegislationDraftVersions.hasChanged()) {
+        return confirm(App.LegislationDraftVersions.msg());
+      } else {
+        return true;
+      }
+    },
+  };
+
+  $(document).on("turbolinks:before-visit", App.LegislationDraftVersions.checkChanges);
+}).call(this);

app/assets/javascripts/watch_form_changes.js

@@ -1,32 +0,0 @@
-(function() {
-  "use strict";
-  App.WatchFormChanges = {
-    forms: function() {
-      return $("form[data-watch-changes]");
-    },
-    msg: function() {
-      return $("[data-watch-form-message]").data("watch-form-message");
-    },
-    hasChanged: function() {
-      return App.WatchFormChanges.forms().is(function() {
-        return $(this).serialize() !== $(this).data("watchChanges");
-      });
-    },
-    checkChanges: function() {
-      if (App.WatchFormChanges.hasChanged()) {
-        return confirm(App.WatchFormChanges.msg());
-      } else {
-        return true;
-      }
-    },
-    initialize: function() {
-      if (App.WatchFormChanges.forms().length === 0 || App.WatchFormChanges.msg() === undefined) {
-        return;
-      }
-      $(document).off("turbolinks:before-visit").on("turbolinks:before-visit", App.WatchFormChanges.checkChanges);
-      App.WatchFormChanges.forms().each(function() {
-        $(this).data("watchChanges", $(this).serialize());
-      });
-    }
-  };
-}).call(this);

app/views/admin/legislation/draft_versions/_form.html.erb

@@ -1,6 +1,6 @@
 <%= render "shared/globalize_locales", resource: @draft_version %>
 
-<%= translatable_form_for [:admin, @process, @draft_version], url: url, html: { data: { watch_changes: true }} do |f| %>
+<%= translatable_form_for [:admin, @process, @draft_version], url: url do |f| %>
 
   <%= render "shared/errors", resource: @draft_version %>
 

app/views/admin/legislation/draft_versions/edit.html.erb

@@ -12,7 +12,7 @@
     <%= render "admin/legislation/processes/subnav", process: @process, active: "draft_versions" %>
 
     <div class="small-12 column">
-      <div class="callout warning" style="display: none;">
+      <div class="callout warning" data-markdown-changes-message="<%= I18n.t("admin.legislation.draft_versions.edit.markdown_changes_message") %>" style="display: none;">
         <%= t("admin.legislation.draft_versions.edit.warning") %>
       </div>
     </div>

app/views/admin/legislation/homepages/_form.html.erb

@@ -3,7 +3,7 @@
            display_style: lambda { |locale| enable_translation_style(@process, locale) },
            manage_languages: false %>
 
-<%= translatable_form_for [:admin, @process], url: url, html: { data: { watch_changes: true } } do |f| %>
+<%= translatable_form_for [:admin, @process], url: url do |f| %>
 
   <%= render "shared/errors", resource: @process %>
   <div class="row">

app/views/admin/legislation/processes/_form.html.erb

@@ -1,6 +1,6 @@
 <%= render "shared/globalize_locales", resource: @process %>
 
-<%= translatable_form_for [:admin, @process], html: { data: { watch_changes: true } } do |f| %>
+<%= translatable_form_for [:admin, @process] do |f| %>
 
   <%= render "shared/errors", resource: @process %>
 

app/views/admin/legislation/proposals/_form.html.erb

@@ -1,4 +1,4 @@
-<%= form_for [:admin, @process], html: { data: { watch_changes: true } } do |f| %>
+<%= form_for [:admin, @process] do |f| %>
 
   <%= render "shared/errors", resource: @process %>
 

app/views/admin/legislation/questions/_form.html.erb

@@ -1,6 +1,6 @@
 <%= render "shared/globalize_locales", resource: @question %>
 
-<%= translatable_form_for [:admin, @process, @question], url: url, html: { data: { watch_changes: true } } do |f| %>
+<%= translatable_form_for [:admin, @process, @question], url: url do |f| %>
 
   <%= render "shared/errors", resource: @question %>
 

app/views/admin/site_customization/content_blocks/_form_content_block.html.erb

@@ -1,4 +1,4 @@
-<%= form_for [:admin, @content_block], html: { class: "edit_page", data: { watch_changes: true } } do |f| %>
+<%= form_for [:admin, @content_block], html: { class: "edit_page" } do |f| %>
 
   <%= render "shared/errors", resource: @content_block %>
 

app/views/admin/site_customization/pages/_form.html.erb

@@ -1,6 +1,6 @@
 <%= render "shared/globalize_locales", resource: @page %>
 
-<%= translatable_form_for [:admin, @page], html: { class: "edit_page", data: { watch_changes: true } } do |f| %>
+<%= translatable_form_for [:admin, @page], html: { class: "edit_page" } do |f| %>
   <%= render "shared/errors", resource: @page %>
 
   <div class="row">

app/views/layouts/admin.html.erb

@@ -6,7 +6,7 @@
     <%= content_for :head %>
   </head>
 
-  <body class="admin" data-watch-form-message="<%= I18n.t("layouts.admin.watch_form_message") %>">
+  <body class="admin">
     <div class="off-canvas-wrapper">
       <div class="off-canvas-wrapper-inner" data-off-canvas-wrapper>
         <div class="off-canvas position-left" id="offCanvas" data-off-canvas>

config/locales/en/admin.yml

@@ -553,6 +553,7 @@ en:
           back: Back
           submit_button: Save changes
           warning: You've edited the text, don't forget to click on Save to permanently save the changes.
+          markdown_changes_message: "You've edited the text without saving it. Do you confirm to leave the page?"
         form:
           title: 'Editing <span class="strong">%{draft_version_title}</span> from the process <span class="strong">%{process_title}</span>'
           launch_text_editor: Launch text editor

config/locales/en/general.yml

@@ -252,8 +252,6 @@ en:
           other: You have %{count} new notifications
         notifications: Notifications
         no_notifications: "You don't have new notifications"
-    admin:
-      watch_form_message: "You have unsaved changes. Do you confirm to leave the page?"
   notifications:
     index:
       empty_notifications: You don't have new notifications.

config/locales/es/admin.yml

@@ -552,6 +552,7 @@ es:
           back: Volver
           submit_button: Guardar cambios
           warning: Ojo, has editado el texto. Para conservar de forma permanente los cambios, no te olvides de hacer click en Guardar.
+          markdown_changes_message: "Ojo, has editado el texto; perderás los cambios si abandonas la página. ¿Seguro que quieres continuar?"
         form:
           title: 'Editando <span class="strong">%{draft_version_title}</span> del proceso <span class="strong">%{process_title}</span>'
           launch_text_editor: Lanzar editor de texto

config/locales/es/general.yml

@@ -252,8 +252,6 @@ es:
           other: Tienes %{count} notificaciones nuevas
         notifications: Notificaciones
         no_notifications: "No tienes notificaciones nuevas"
-    admin:
-      watch_form_message: "Has realizado cambios que no han sido guardados. ¿Seguro que quieres abandonar la página?"
   notifications:
     index:
       empty_notifications: No tienes notificaciones nuevas.

spec/system/admin/legislation/draft_versions_spec.rb

@@ -90,4 +90,61 @@ describe "Admin legislation draft versions" do
       expect(page).to have_content "Version 1b"
     end
   end
+
+  context "Changing content with the markdown editor", :js do
+    let(:prompt) { "You've edited the text without saving it. Do you confirm to leave the page?" }
+    let(:version) { create(:legislation_draft_version, body: "Version 1") }
+    let(:path) do
+      edit_admin_legislation_process_draft_version_path(version.process, version)
+    end
+
+    scenario "asks for confimation when the content is modified" do
+      visit path
+      fill_in_markdown_editor "Text", with: "Version 1b"
+
+      dismiss_confirm(prompt) do
+        click_link "Proposals", match: :first
+      end
+
+      expect(page).to have_current_path(path)
+    end
+
+    scenario "asks for confimation after the page is restored from browser history" do
+      visit path
+      fill_in_markdown_editor "Text", with: "Version 1b"
+
+      accept_confirm(prompt) do
+        click_link "Proposals", match: :first
+      end
+
+      expect(page).to have_css("h2", text: "Proposals")
+
+      go_back
+
+      expect(page).to have_content version.process.title
+
+      accept_confirm(prompt) do
+        click_link "Proposals", match: :first
+      end
+
+      expect(page).to have_css("h2", text: "Proposals")
+    end
+
+    scenario "does not ask for confirmation when restoring the original content" do
+      visit path
+      fill_in_markdown_editor "Text", with: "Version 1b"
+
+      accept_confirm(prompt) do
+        click_link "Proposals", match: :first
+      end
+
+      expect(page).to have_css("h2", text: "Proposals")
+
+      go_back
+      fill_in_markdown_editor "Text", with: "Version 1"
+      click_link "Proposals", match: :first
+
+      expect(page).to have_css("h2", text: "Proposals")
+    end
+  end
 end