From 4fdbfda20fb17ab774426afca77e71266db2b597 Mon Sep 17 00:00:00 2001 From: Prabin Poudel <15196941+coolprobn@users.noreply.github.com> Date: Thu, 17 Oct 2024 08:25:08 +0545 Subject: [PATCH 1/5] feature: Alert users when form has unsaved changes --- .../views/resource_edit_component.html.erb | 1 + app/javascript/js/controllers.js | 2 + .../form_exit_prompt_controller.js | 58 +++++++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 app/javascript/js/controllers/form_exit_prompt_controller.js diff --git a/app/components/avo/views/resource_edit_component.html.erb b/app/components/avo/views/resource_edit_component.html.erb index 950e2b331f..976f50c1ae 100644 --- a/app/components/avo/views/resource_edit_component.html.erb +++ b/app/components/avo/views/resource_edit_component.html.erb @@ -17,6 +17,7 @@ html: { novalidate: true }, + data: { controller: "form-exit-prompt", form_exit_prompt_target: "form" }, multipart: true do |form| %> <%= render Avo::ReferrerParamsComponent.new back_path: back_path %> <%= content_tag :div, class: "space-y-12" do %> diff --git a/app/javascript/js/controllers.js b/app/javascript/js/controllers.js index ff30529b03..f29cacfab2 100644 --- a/app/javascript/js/controllers.js +++ b/app/javascript/js/controllers.js @@ -14,6 +14,7 @@ import DateFieldController from './controllers/fields/date_field_controller' import DateTimeFilterController from './controllers/date_time_filter_controller' import EasyMdeController from './controllers/fields/easy_mde_controller' import FilterController from './controllers/filter_controller' +import FormExitPromptController from './controllers/form_exit_prompt_controller' import HiddenInputController from './controllers/hidden_input_controller' import InputAutofocusController from './controllers/input_autofocus_controller' import ItemSelectAllController from './controllers/item_select_all_controller' @@ -56,6 +57,7 @@ application.register('copy-to-clipboard', CopyToClipboardController) application.register('dashboard-card', DashboardCardController) application.register('date-time-filter', DateTimeFilterController) application.register('filter', FilterController) +application.register('form-exit-prompt', FormExitPromptController) application.register('panel-refresh', PanelRefreshController) application.register('hidden-input', HiddenInputController) application.register('input-autofocus', InputAutofocusController) diff --git a/app/javascript/js/controllers/form_exit_prompt_controller.js b/app/javascript/js/controllers/form_exit_prompt_controller.js new file mode 100644 index 0000000000..2db4ff59a7 --- /dev/null +++ b/app/javascript/js/controllers/form_exit_prompt_controller.js @@ -0,0 +1,58 @@ +import { Controller } from '@hotwired/stimulus' + +export default class extends Controller { + static targets = ['form'] + + connect() { + this.isDirty = false + + // for select tags + this.formTarget.addEventListener('change', this.trackChanges.bind(this)) + // for all other input fields + this.formTarget.addEventListener('input', this.trackChanges.bind(this)) + + // in most cases this event will be triggered because Turbo prevents full page reload on navigation + window.addEventListener( + 'turbo:before-visit', + this.preventTurboNavigation.bind(this), + ) + window.addEventListener('beforeunload', this.preventFullPageNavigation.bind(this)) + } + + disconnect() { + window.removeEventListener( + 'turbo:before-visit', + this.preventTurboNavigation.bind(this), + ) + window.removeEventListener( + 'beforeunload', + this.preventFullPageNavigation.bind(this), + ) + } + + trackChanges() { + this.isDirty = true + } + + preventTurboNavigation(event) { + if (this.isDirty) { + const message = 'Are you sure you want to navigate away from the page? You will lose all your changes.' + + if (window.confirm(message)) { + this.isDirty = false + } else { + event.preventDefault() + } + } + } + + preventFullPageNavigation(event) { + if (this.isDirty) { + event.preventDefault() + + // for legacy browsers support + // see: https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event + event.returnValue = 'Are you sure you want to navigate away from the page? You will lose all your changes.' + } + } +} From b4122e15a77d9acf7b6dee2d04c2504067e4ab3c Mon Sep 17 00:00:00 2001 From: Prabin Poudel <15196941+coolprobn@users.noreply.github.com> Date: Sun, 20 Oct 2024 11:08:51 +0545 Subject: [PATCH 2/5] Handle form undirtying --- .../fields/key_value_controller.js | 2 +- .../form_exit_prompt_controller.js | 47 ++++++++++++++++++- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/app/javascript/js/controllers/fields/key_value_controller.js b/app/javascript/js/controllers/fields/key_value_controller.js index dfa0b05255..dfcf13aaff 100644 --- a/app/javascript/js/controllers/fields/key_value_controller.js +++ b/app/javascript/js/controllers/fields/key_value_controller.js @@ -73,7 +73,7 @@ export default class extends Controller { result = Object.assign(...this.fieldValue.map(([key, val]) => ({ [key]: val }))) } this.inputTarget.innerText = JSON.stringify(result) - this.inputTarget.dispatchEvent(new Event('input')) + this.inputTarget.dispatchEvent(new Event('input', { bubbles: true })) } updateKeyValueComponent() { diff --git a/app/javascript/js/controllers/form_exit_prompt_controller.js b/app/javascript/js/controllers/form_exit_prompt_controller.js index 2db4ff59a7..64b195ed7e 100644 --- a/app/javascript/js/controllers/form_exit_prompt_controller.js +++ b/app/javascript/js/controllers/form_exit_prompt_controller.js @@ -5,6 +5,10 @@ export default class extends Controller { connect() { this.isDirty = false + this.skipFormStateEvaluation = false + + this.initialFormState = this.getFormState() + this.currentFormState = this.getFormState() // for select tags this.formTarget.addEventListener('change', this.trackChanges.bind(this)) @@ -30,16 +34,53 @@ export default class extends Controller { ) } - trackChanges() { - this.isDirty = true + getFormState() { + const formState = {} + const formFieldsArray = [...this.formTarget.querySelectorAll('input, textarea, select')] + const formFieldsWithIdentifier = formFieldsArray.filter((item) => Boolean(item.id)) + + formFieldsWithIdentifier.forEach((item) => { + let { value } = item + + if (item.type === 'checkbox') { + value = item.checked + } + + formState[item.id] = value + }) + + return formState + } + + trackChanges(event) { + const { target: { id, type: fieldType, checked } } = event + + let { target: { value } } = event + + if (fieldType === 'checkbox') { + value = checked + } + + this.currentFormState[id] = value + } + + evaluateFormState() { + if (this.skipFormStateEvaluation) return + + const isFormDirty = Object.keys(this.initialFormState).some((key) => this.initialFormState[key] !== this.currentFormState[key]) + + this.isDirty = isFormDirty } preventTurboNavigation(event) { + this.evaluateFormState() + if (this.isDirty) { const message = 'Are you sure you want to navigate away from the page? You will lose all your changes.' if (window.confirm(message)) { this.isDirty = false + this.skipFormStateEvaluation = true } else { event.preventDefault() } @@ -47,6 +88,8 @@ export default class extends Controller { } preventFullPageNavigation(event) { + this.evaluateFormState() + if (this.isDirty) { event.preventDefault() From 80a0ad62332463f9433b17abe8778a6332d20354 Mon Sep 17 00:00:00 2001 From: Prabin Poudel <15196941+coolprobn@users.noreply.github.com> Date: Tue, 22 Oct 2024 18:53:04 +0545 Subject: [PATCH 3/5] Fix the issue of form dirty state not working for markdowns and belongs_to fields --- .../avo/fields/trix_field/edit_component.html.erb | 5 ++++- .../js/controllers/fields/code_field_controller.js | 2 +- .../js/controllers/fields/easy_mde_controller.js | 8 +++++++- .../js/controllers/fields/tiptap_field_controller.js | 2 ++ .../js/controllers/fields/trix_field_controller.js | 6 +++++- .../js/controllers/form_exit_prompt_controller.js | 8 +++++++- 6 files changed, 26 insertions(+), 5 deletions(-) diff --git a/app/components/avo/fields/trix_field/edit_component.html.erb b/app/components/avo/fields/trix_field/edit_component.html.erb index 01fdae224e..12d83525a4 100644 --- a/app/components/avo/fields/trix_field/edit_component.html.erb +++ b/app/components/avo/fields/trix_field/edit_component.html.erb @@ -19,7 +19,10 @@ <%= @form.text_area @field.id, value: @field.value.try(:to_trix_html) || @field.value, class: classes("w-full hidden"), - data: @field.get_html(:data, view: view, element: :input), + data: { + "trix-field-target": "textarea", + **@field.get_html(:data, view: view, element: :input) + }, disabled: disabled?, id: trix_id, placeholder: @field.placeholder, diff --git a/app/javascript/js/controllers/fields/code_field_controller.js b/app/javascript/js/controllers/fields/code_field_controller.js index c8bcfd5ffa..593a9c4e19 100644 --- a/app/javascript/js/controllers/fields/code_field_controller.js +++ b/app/javascript/js/controllers/fields/code_field_controller.js @@ -38,7 +38,7 @@ export default class extends Controller { CodeMirror.fromTextArea(this.elementTarget, options).on('change', (cm) => { // Add this innerText change and dispatch an event to allow stimulus to pick up the input event. vm.elementTarget.innerText = cm.getValue() - vm.elementTarget.dispatchEvent(new Event('input')) + vm.elementTarget.dispatchEvent(new Event('input', { bubbles: true })) }) }, 1) } diff --git a/app/javascript/js/controllers/fields/easy_mde_controller.js b/app/javascript/js/controllers/fields/easy_mde_controller.js index 3d504af39c..a49305a139 100644 --- a/app/javascript/js/controllers/fields/easy_mde_controller.js +++ b/app/javascript/js/controllers/fields/easy_mde_controller.js @@ -20,7 +20,7 @@ export default class extends Controller { const options = { element: this.elementTarget, spellChecker: this.componentOptions.spell_checker, - autoRefresh: { delay: 500}, + autoRefresh: { delay: 500 }, } if (this.view === 'show') { @@ -29,6 +29,12 @@ export default class extends Controller { } const easyMde = new EasyMDE(options) + + easyMde.codemirror.on('change', () => { + this.elementTarget.value = easyMde.value() + this.elementTarget.dispatchEvent(new Event('input', { bubbles: true })) + }) + if (this.view === 'show') { easyMde.codemirror.options.readOnly = true } diff --git a/app/javascript/js/controllers/fields/tiptap_field_controller.js b/app/javascript/js/controllers/fields/tiptap_field_controller.js index 82f5391965..b453b473ff 100644 --- a/app/javascript/js/controllers/fields/tiptap_field_controller.js +++ b/app/javascript/js/controllers/fields/tiptap_field_controller.js @@ -70,6 +70,8 @@ export default class extends Controller { onUpdate = () => { this.inputTarget.value = this.editor.getHTML() + + this.inputTarget.dispatchEvent(new Event('input', { bubbles: true })) } onSelectionUpdate = () => { diff --git a/app/javascript/js/controllers/fields/trix_field_controller.js b/app/javascript/js/controllers/fields/trix_field_controller.js index 45d776d818..12dd104bf0 100644 --- a/app/javascript/js/controllers/fields/trix_field_controller.js +++ b/app/javascript/js/controllers/fields/trix_field_controller.js @@ -6,7 +6,7 @@ import { Controller } from '@hotwired/stimulus' import { castBoolean } from '../../helpers/cast_boolean' export default class extends Controller { - static targets = ['editor', 'controller'] + static targets = ['editor', 'controller', 'textarea'] static values = { resourceName: String, @@ -33,6 +33,10 @@ export default class extends Controller { } connect() { + this.editorTarget.addEventListener('trix-change', () => { + this.textareaTarget.dispatchEvent(new Event('input', { bubbles: true })) + }) + if (this.attachmentsDisabledValue) { // Remove the attachments button this.controllerTarget.querySelector('.trix-button-group--file-tools').remove() diff --git a/app/javascript/js/controllers/form_exit_prompt_controller.js b/app/javascript/js/controllers/form_exit_prompt_controller.js index 64b195ed7e..5f6c345af7 100644 --- a/app/javascript/js/controllers/form_exit_prompt_controller.js +++ b/app/javascript/js/controllers/form_exit_prompt_controller.js @@ -68,11 +68,17 @@ export default class extends Controller { if (this.skipFormStateEvaluation) return const isFormDirty = Object.keys(this.initialFormState).some((key) => this.initialFormState[key] !== this.currentFormState[key]) + const isNewFieldAdded = Object.keys(this.currentFormState).length > Object.keys(this.initialFormState).length - this.isDirty = isFormDirty + this.isDirty = isFormDirty || isNewFieldAdded } preventTurboNavigation(event) { + // don't intercept if modals + if (event.detail.url.includes('/new')) { + return + } + this.evaluateFormState() if (this.isDirty) { From c9af2cce9a7392b7d1b7e54fe115ddb9f40b818c Mon Sep 17 00:00:00 2001 From: Prabin Poudel <15196941+coolprobn@users.noreply.github.com> Date: Fri, 15 Nov 2024 09:28:49 +0545 Subject: [PATCH 4/5] Add system tests for the feature --- .../form_exit_prompt_controller.js | 33 +++++-- spec/system/form_exit_prompt_spec.rb | 93 +++++++++++++++++++ 2 files changed, 119 insertions(+), 7 deletions(-) create mode 100644 spec/system/form_exit_prompt_spec.rb diff --git a/app/javascript/js/controllers/form_exit_prompt_controller.js b/app/javascript/js/controllers/form_exit_prompt_controller.js index 5f6c345af7..d89a57e97b 100644 --- a/app/javascript/js/controllers/form_exit_prompt_controller.js +++ b/app/javascript/js/controllers/form_exit_prompt_controller.js @@ -5,7 +5,8 @@ export default class extends Controller { connect() { this.isDirty = false - this.skipFormStateEvaluation = false + this.isFormSubmitting = false + this.currentLocationUrl = window.location.href this.initialFormState = this.getFormState() this.currentFormState = this.getFormState() @@ -21,6 +22,9 @@ export default class extends Controller { this.preventTurboNavigation.bind(this), ) window.addEventListener('beforeunload', this.preventFullPageNavigation.bind(this)) + + this.formTarget.addEventListener('turbo:submit-start', this.handleFormSubmitStart.bind(this)) + this.formTarget.addEventListener('turbo:submit-end', this.handleFormSubmitEnd.bind(this)) } disconnect() { @@ -65,17 +69,33 @@ export default class extends Controller { } evaluateFormState() { - if (this.skipFormStateEvaluation) return - const isFormDirty = Object.keys(this.initialFormState).some((key) => this.initialFormState[key] !== this.currentFormState[key]) + // for key value fields which are not present in initial state for new form const isNewFieldAdded = Object.keys(this.currentFormState).length > Object.keys(this.initialFormState).length this.isDirty = isFormDirty || isNewFieldAdded } + handleFormSubmitStart() { + this.isFormSubmitting = true + } + + handleFormSubmitEnd(event) { + if (event.detail.success) { + this.resetState() + } + } + + resetState() { + this.isDirty = false + this.isFormSubmitting = false + this.initialFormState = {} + this.currentFormState = {} + } + preventTurboNavigation(event) { - // don't intercept if modals - if (event.detail.url.includes('/new')) { + // don't intercept if URL doesn't change e.g. modals OR when form is submitting + if (event.detail.url === this.currentLocationUrl || this.isFormSubmitting) { return } @@ -85,8 +105,7 @@ export default class extends Controller { const message = 'Are you sure you want to navigate away from the page? You will lose all your changes.' if (window.confirm(message)) { - this.isDirty = false - this.skipFormStateEvaluation = true + this.resetState() } else { event.preventDefault() } diff --git a/spec/system/form_exit_prompt_spec.rb b/spec/system/form_exit_prompt_spec.rb new file mode 100644 index 0000000000..99312025c2 --- /dev/null +++ b/spec/system/form_exit_prompt_spec.rb @@ -0,0 +1,93 @@ +require "rails_helper" + +RSpec.describe "form_exit_prompt", type: :system do + let(:city) { create :city } + + context "when navigating away with unsaved changes" do + it "prompts the user with a confirmation message and prevents navigation if the user cancels" do + visit "/admin/resources/cities" + click_on "Create new city" + fill_in "city_name", with: city.name + + message = dismiss_prompt { click_on "Comments" } + + expect(message).to eq( + "Are you sure you want to navigate away from the page? You will lose all your changes." + ) + expect(page).to have_current_path("/admin/resources/cities/new") + end + + it "allows navigation if the user confirms" do + visit "/admin/resources/cities/#{city.id}" + click_on "Edit" + fill_in "city_name", with: "#{city.name} updated" + accept_prompt { click_on "Comments" } + + expect(page).to have_current_path("/admin/resources/comments") + end + end + + context "when submitting the form" do + it "does not prompt the user with a confirmation message" do + visit "/admin/resources/cities" + click_on "Create new city" + fill_in "city_name", with: "New City" + click_button "Save" + + expect(page).to have_current_path(%r{/admin/resources/cities/\w+$}) + end + end + + context "when navigating away without changing anything in the form" do + it "does not prompt the user with a confirmation message" do + visit "/admin/resources/cities" + click_on "Create new city" + click_on "Comments" + + expect(page).to have_current_path("/admin/resources/comments") + end + end + + context "when reloading the page" do + it "prompts the user with a confirmation message" do + visit "/admin/resources/cities" + click_on "Create new city" + fill_in "city_name", with: city.name + + message = accept_prompt { page.refresh } + + expect(message).to eq("") + end + end + + context "when form is reverted to clean state" do + it "does not prompt the user with a confirmation message" do + visit "/admin/resources/cities/#{city.id}" + click_on "Edit" + fill_in "city_name", with: "#{city.name} updated" + + message = dismiss_prompt { click_on "Cancel" } + + expect(message).to eq( + "Are you sure you want to navigate away from the page? You will lose all your changes." + ) + + fill_in "city_name", with: city.name + click_on "Cancel" + + expect(page).to have_current_path(%r{/admin/resources/cities/\w+$}) + end + end + + context "when opening modals" do + it "does not prompt the user with a confirmation message" do + visit "/admin/resources/cities/#{city.id}" + click_on "Edit" + fill_in "city_name", with: "#{city.name} updated" + click_on "Actions" + click_on "Dummy action city resource" + + expect(page).to have_current_path(%r{/admin/resources/cities/\w+/edit}) + end + end +end From 744a3eb3b4fd8b6574c3c4b5eea5b9d3104cea67 Mon Sep 17 00:00:00 2001 From: Prabin Poudel <15196941+coolprobn@users.noreply.github.com> Date: Fri, 15 Nov 2024 09:40:55 +0545 Subject: [PATCH 5/5] Make code climate happy for the cognitive threshold issue in preventTurboNavigation function --- .../controllers/form_exit_prompt_controller.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/app/javascript/js/controllers/form_exit_prompt_controller.js b/app/javascript/js/controllers/form_exit_prompt_controller.js index d89a57e97b..094c89e714 100644 --- a/app/javascript/js/controllers/form_exit_prompt_controller.js +++ b/app/javascript/js/controllers/form_exit_prompt_controller.js @@ -93,6 +93,16 @@ export default class extends Controller { this.currentFormState = {} } + handleDirtyFormNavigation(event) { + const message = 'Are you sure you want to navigate away from the page? You will lose all your changes.' + + if (window.confirm(message)) { + this.resetState() + } else { + event.preventDefault() + } + } + preventTurboNavigation(event) { // don't intercept if URL doesn't change e.g. modals OR when form is submitting if (event.detail.url === this.currentLocationUrl || this.isFormSubmitting) { @@ -102,13 +112,7 @@ export default class extends Controller { this.evaluateFormState() if (this.isDirty) { - const message = 'Are you sure you want to navigate away from the page? You will lose all your changes.' - - if (window.confirm(message)) { - this.resetState() - } else { - event.preventDefault() - } + this.handleDirtyFormNavigation(event) } }