From fcbf0b9637d0b76b876626e862c1f2281fb568ca Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 10 Oct 2024 17:41:29 +0200 Subject: [PATCH 1/2] fix: Loading state and double renders - Add a loading state in the Match Voucher tab when the matching vouchers take time to arrive - Add a loading state for the transaction summary above the table, which should just be 0 values - fix: Avoid calling `populate_matching_vouchers` twice on changing the match tab filters --- .../actions_panel/match_tab.js | 68 +++++++++++-------- 1 file changed, 40 insertions(+), 28 deletions(-) diff --git a/banking/public/js/bank_reconciliation_beta/actions_panel/match_tab.js b/banking/public/js/bank_reconciliation_beta/actions_panel/match_tab.js index 832d3b1e..16828a02 100644 --- a/banking/public/js/bank_reconciliation_beta/actions_panel/match_tab.js +++ b/banking/public/js/bank_reconciliation_beta/actions_panel/match_tab.js @@ -16,19 +16,31 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { }); this.match_field_group.make() - this.summary_empty_state(); await this.populate_matching_vouchers(); } summary_empty_state() { - let summary_field = this.match_field_group.get_field("transaction_amount_summary").$wrapper; - summary_field.append( - `
-
` + this.render_transaction_amount_summary(0, 0, 0, this.transaction.currency); + } + + matching_vouchers_empty_state() { + this.match_field_group.get_field("vouchers").$wrapper.empty(); + this.match_field_group.get_field("vouchers").$wrapper.append( + `
${__("Loading ...")}
` ); } - async populate_matching_vouchers() { + async populate_matching_vouchers(event_obj) { + if (event_obj && event_obj.type === "input") { + // `bind_change_event` in `data.js` triggers both an input and change event + // This triggers the `populate_matching_vouchers` twice on clicking on filters + // Since the input event is debounced, we can ignore it for a checkbox + return; + } + + this.summary_empty_state(); + this.matching_vouchers_empty_state(); + let filter_fields = this.match_field_group.get_values(); let document_types = Object.keys(filter_fields).filter(field => filter_fields[field] === 1); @@ -261,8 +273,8 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { fieldname: "payment_entry", fieldtype: "Check", default: filters_state.payment_entry, - onchange: () => { - this.populate_matching_vouchers(); + onchange: (e) => { + this.populate_matching_vouchers(e); } }, { @@ -270,8 +282,8 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { fieldname: "journal_entry", fieldtype: "Check", default: filters_state.journal_entry, - onchange: () => { - this.populate_matching_vouchers(); + onchange: (e) => { + this.populate_matching_vouchers(e); } }, { @@ -282,8 +294,8 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { fieldname: "purchase_invoice", fieldtype: "Check", default: filters_state.purchase_invoice, - onchange: () => { - this.populate_matching_vouchers(); + onchange: (e) => { + this.populate_matching_vouchers(e); } }, { @@ -291,8 +303,8 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { fieldname: "sales_invoice", fieldtype: "Check", default: filters_state.sales_invoice, - onchange: () => { - this.populate_matching_vouchers(); + onchange: (e) => { + this.populate_matching_vouchers(e); } }, { @@ -303,8 +315,8 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { fieldname: "loan_repayment", fieldtype: "Check", default: filters_state.loan_repayment, - onchange: () => { - this.populate_matching_vouchers(); + onchange: (e) => { + this.populate_matching_vouchers(e); } }, { @@ -312,8 +324,8 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { fieldname: "loan_disbursement", fieldtype: "Check", default: filters_state.loan_disbursement, - onchange: () => { - this.populate_matching_vouchers(); + onchange: (e) => { + this.populate_matching_vouchers(e); } }, { @@ -324,8 +336,8 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { fieldname: "expense_claim", fieldtype: "Check", default: filters_state.expense_claim, - onchange: () => { - this.populate_matching_vouchers(); + onchange: (e) => { + this.populate_matching_vouchers(e); } }, { @@ -333,8 +345,8 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { fieldname: "bank_transaction", fieldtype: "Check", default: filters_state.bank_transaction, - onchange: () => { - this.populate_matching_vouchers(); + onchange: (e) => { + this.populate_matching_vouchers(e); } }, { @@ -345,8 +357,8 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { fieldname: "exact_match", fieldtype: "Check", default: filters_state.exact_match, - onchange: () => { - this.populate_matching_vouchers(); + onchange: (e) => { + this.populate_matching_vouchers(e); } }, { @@ -357,8 +369,8 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { fieldname: "exact_party_match", fieldtype: "Check", default: this.transaction.party_type && this.transaction.party ? 1 : 0, - onchange: () => { - this.populate_matching_vouchers(); + onchange: (e) => { + this.populate_matching_vouchers(e); }, read_only: !Boolean(this.transaction.party_type && this.transaction.party) }, @@ -370,8 +382,8 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { fieldname: "unpaid_invoices", fieldtype: "Check", default: filters_state.unpaid_invoices, - onchange: () => { - this.populate_matching_vouchers(); + onchange: (e) => { + this.populate_matching_vouchers(e); }, depends_on: "eval: doc.sales_invoice || doc.purchase_invoice || doc.expense_claim", }, From 0735fb0701629e3e7ae491881888281635e1ec71 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Wed, 16 Oct 2024 11:00:31 +0200 Subject: [PATCH 2/2] refactor: show frozen datatable while loading --- .../actions_panel/match_tab.js | 63 +++++++++---------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/banking/public/js/bank_reconciliation_beta/actions_panel/match_tab.js b/banking/public/js/bank_reconciliation_beta/actions_panel/match_tab.js index 16828a02..fad03e06 100644 --- a/banking/public/js/bank_reconciliation_beta/actions_panel/match_tab.js +++ b/banking/public/js/bank_reconciliation_beta/actions_panel/match_tab.js @@ -23,13 +23,6 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { this.render_transaction_amount_summary(0, 0, 0, this.transaction.currency); } - matching_vouchers_empty_state() { - this.match_field_group.get_field("vouchers").$wrapper.empty(); - this.match_field_group.get_field("vouchers").$wrapper.append( - `
${__("Loading ...")}
` - ); - } - async populate_matching_vouchers(event_obj) { if (event_obj && event_obj.type === "input") { // `bind_change_event` in `data.js` triggers both an input and change event @@ -39,7 +32,8 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { } this.summary_empty_state(); - this.matching_vouchers_empty_state(); + this.render_data_table(); + this.actions_table.freeze(); let filter_fields = this.match_field_group.get_values(); let document_types = Object.keys(filter_fields).filter(field => filter_fields[field] === 1); @@ -47,7 +41,8 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { this.update_filters_in_state(document_types); let vouchers = await this.get_matching_vouchers(document_types); - this.render_data_table(vouchers); + this.set_table_data(vouchers); + this.actions_table.unfreeze(); let transaction_amount = this.transaction.withdrawal || this.transaction.deposit; this.render_transaction_amount_summary( @@ -82,7 +77,32 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { return vouchers || []; } - render_data_table(vouchers) { + render_data_table() { + const datatable_options = { + columns: this.get_data_table_columns(), + data: [], + dynamicRowHeight: true, + checkboxColumn: true, + inlineFilters: true, + layout: "fluid", + serialNoColumn: false, + freezeMessage: __("Loading..."), + }; + + this.actions_table = new frappe.DataTable( + this.match_field_group.get_field("vouchers").$wrapper[0], + datatable_options + ); + + // Highlight first row + this.actions_table.style.setStyle( + ".dt-cell[data-row-index='0']", {backgroundColor: '#F4FAEE'} + ); + + this.bind_row_check_event(); + } + + set_table_data(vouchers) { this.summary_data = {}; let table_data = vouchers.map((row) => { return [ @@ -127,28 +147,7 @@ erpnext.accounts.bank_reconciliation.MatchTab = class MatchTab { ]; }); - const datatable_options = { - columns: this.get_data_table_columns(), - data: table_data, - dynamicRowHeight: true, - checkboxColumn: true, - inlineFilters: true, - layout: "fluid", - serialNoColumn: false, - }; - - - this.actions_table = new frappe.DataTable( - this.match_field_group.get_field("vouchers").$wrapper[0], - datatable_options - ); - - // Highlight first row - this.actions_table.style.setStyle( - ".dt-cell[data-row-index='0']", {backgroundColor: '#F4FAEE'} - ); - - this.bind_row_check_event(); + this.actions_table.refresh(table_data, this.get_data_table_columns()); } bind_row_check_event() {