From cbcf921dfb2d6caf011c95a51792f9258646580a Mon Sep 17 00:00:00 2001 From: Ejaaz Khan Date: Fri, 24 Jan 2025 17:33:51 +0530 Subject: [PATCH 1/5] fix: recursive call of handle scrollbar function --- frappe/public/js/frappe/form/grid.js | 51 +++++++++++++++++++++++ frappe/public/js/frappe/form/grid_row.js | 53 +++--------------------- 2 files changed, 57 insertions(+), 47 deletions(-) diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index 306b43f8c8..0614a19007 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -41,6 +41,57 @@ export default class Grid { this.is_grid = true; this.debounced_refresh = this.refresh.bind(this); this.debounced_refresh = frappe.utils.debounce(this.debounced_refresh, 100); + this.handle_scroll_bar(); + this.setup_tab_listeners(); + } + + setup_tab_listeners() { + $(".nav-link").on("shown.bs.tab", () => { + this.handle_scroll_bar(); + }); + } + + handle_scroll_bar() { + console.log("handle_scroll_bar"); + + $(document).ready(function () { + let $scrollBar = $(".grid-scroll-bar"); + let form_grid = $(".form-grid"); + let grid_scroll_bar_rows = $(".grid-scroll-bar-rows"); + let parent_field_name = ""; + let grid_width = 0; + let grid_body_width = 0; + form_grid.each((grid_index, grid) => { + console.log("grid", grid, parent_field_name); + + parent_field_name = $(grid) + .closest("[data-fieldname][data-fieldtype]") + .attr("data-fieldname"); + grid_width = $($(grid)[0]).width(); + grid_body_width = $( + 'div[data-fieldname="' + parent_field_name + '"] .grid-body' + ).width(); + console.log("grid_width", grid_width, grid_body_width); + + $('div[data-fieldname="' + parent_field_name + '"] .grid-scroll-bar').width( + grid_width + ); + $('div[data-fieldname="' + parent_field_name + '"] .grid-scroll-bar-rows').width( + grid_body_width + ); + }); + + // Make sure the grid container is scrollable + $scrollBar.on("scroll", function (event) { + grid_scroll_bar_rows = $(event.currentTarget).closest(".grid-scroll-bar-rows"); + + var scroll_left = $(this).scrollLeft(); + + // Sync the form grid's left position with the scroll bar + form_grid.css("position", "relative"); + form_grid.css("left", -scroll_left + "px"); + }); + }); } get perm() { diff --git a/frappe/public/js/frappe/form/grid_row.js b/frappe/public/js/frappe/form/grid_row.js index 56f594f23b..7f78ba0827 100644 --- a/frappe/public/js/frappe/form/grid_row.js +++ b/frappe/public/js/frappe/form/grid_row.js @@ -10,7 +10,6 @@ export default class GridRow { this.columns_list = []; this.row_check_html = ''; this.make(); - this.setup_tab_listeners(); } make() { let me = this; @@ -409,12 +408,18 @@ export default class GridRow { this.columns = {}; this.update_user_settings_for_grid(); this.grid_settings_dialog.hide(); + setTimeout(() => { + this.grid.handle_scroll_bar(); + }, 500); }); this.grid_settings_dialog.set_secondary_action_label(__("Reset to default")); this.grid_settings_dialog.set_secondary_action(() => { this.reset_user_settings_for_grid(); this.grid_settings_dialog.hide(); + setTimeout(() => { + this.grid.handle_scroll_bar(); + }, 500); }); } @@ -987,8 +992,6 @@ export default class GridRow { $col.field_area = $('
').appendTo($col).toggle(false); $col.static_area = $('
').appendTo($col).html(txt); - this.handle_scroll_bar(); - // set title attribute to see full label for columns in the heading row if (!this.doc) { $col.attr("title", txt); @@ -1004,50 +1007,6 @@ export default class GridRow { return $col; } - handle_scroll_bar() { - $(document).ready(function () { - let $scrollBar = $(".grid-scroll-bar"); - let form_grid = $(".form-grid"); - let grid_scroll_bar_rows = $(".grid-scroll-bar-rows"); - let parent_field_name = ""; - let grid_width = 0; - let grid_body_width = 0; - form_grid.each((grid_index, grid) => { - parent_field_name = $(grid) - .closest("[data-fieldname][data-fieldtype]") - .attr("data-fieldname"); - grid_width = $($(grid)[0]).width(); - grid_body_width = $( - 'div[data-fieldname="' + parent_field_name + '"] .grid-body' - ).width(); - - $('div[data-fieldname="' + parent_field_name + '"] .grid-scroll-bar').width( - grid_width - ); - $('div[data-fieldname="' + parent_field_name + '"] .grid-scroll-bar-rows').width( - grid_body_width - ); - }); - - // Make sure the grid container is scrollable - $scrollBar.on("scroll", function (event) { - grid_scroll_bar_rows = $(event.currentTarget).closest(".grid-scroll-bar-rows"); - - var scroll_left = $(this).scrollLeft(); - - // Sync the form grid's left position with the scroll bar - form_grid.css("position", "relative"); - form_grid.css("left", -scroll_left + "px"); - }); - }); - } - - setup_tab_listeners() { - $(".nav-link").on("shown.bs.tab", () => { - this.handle_scroll_bar(); - }); - } - activate() { this.toggle_editable_row(true); return this; From d01eb3a18e91a7a8fc9161bedaaa594bfbe11a34 Mon Sep 17 00:00:00 2001 From: Ejaaz Khan Date: Fri, 24 Jan 2025 17:45:01 +0530 Subject: [PATCH 2/5] refactor: remove unwanted log code --- frappe/public/js/frappe/form/grid.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index 0614a19007..1ccc7c6e13 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -62,8 +62,6 @@ export default class Grid { let grid_width = 0; let grid_body_width = 0; form_grid.each((grid_index, grid) => { - console.log("grid", grid, parent_field_name); - parent_field_name = $(grid) .closest("[data-fieldname][data-fieldtype]") .attr("data-fieldname"); From e76cb990ab5c11c9a1b132517807443ecfab117f Mon Sep 17 00:00:00 2001 From: Ejaaz Khan Date: Sun, 26 Jan 2025 12:24:46 +0530 Subject: [PATCH 3/5] refactor: add scroll to current element, remove grid loop --- frappe/public/js/frappe/form/grid.js | 46 ++++++++---------------- frappe/public/js/frappe/form/grid_row.js | 8 ++--- 2 files changed, 17 insertions(+), 37 deletions(-) diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index 1ccc7c6e13..87035b425f 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -41,7 +41,6 @@ export default class Grid { this.is_grid = true; this.debounced_refresh = this.refresh.bind(this); this.debounced_refresh = frappe.utils.debounce(this.debounced_refresh, 100); - this.handle_scroll_bar(); this.setup_tab_listeners(); } @@ -50,44 +49,27 @@ export default class Grid { this.handle_scroll_bar(); }); } - handle_scroll_bar() { - console.log("handle_scroll_bar"); + frappe.utils.sleep(500).then(() => { + let form_grid = $(`div[data-fieldname="${this.df.fieldname}"] .form-grid`); + let grid_body_width = $( + `div[data-fieldname="${this.df.fieldname}"] .grid-body` + ).width(); - $(document).ready(function () { - let $scrollBar = $(".grid-scroll-bar"); - let form_grid = $(".form-grid"); - let grid_scroll_bar_rows = $(".grid-scroll-bar-rows"); - let parent_field_name = ""; - let grid_width = 0; - let grid_body_width = 0; - form_grid.each((grid_index, grid) => { - parent_field_name = $(grid) - .closest("[data-fieldname][data-fieldtype]") - .attr("data-fieldname"); - grid_width = $($(grid)[0]).width(); - grid_body_width = $( - 'div[data-fieldname="' + parent_field_name + '"] .grid-body' - ).width(); - console.log("grid_width", grid_width, grid_body_width); + let grid_scroll_bar = $(`div[data-fieldname="${this.df.fieldname}"] .grid-scroll-bar`); + let grid_scroll_bar_rows = $( + `div[data-fieldname="${this.df.fieldname}"] .grid-scroll-bar-rows` + ); - $('div[data-fieldname="' + parent_field_name + '"] .grid-scroll-bar').width( - grid_width - ); - $('div[data-fieldname="' + parent_field_name + '"] .grid-scroll-bar-rows').width( - grid_body_width - ); - }); + grid_scroll_bar.width(form_grid.width()); + grid_scroll_bar_rows.width(grid_body_width); - // Make sure the grid container is scrollable - $scrollBar.on("scroll", function (event) { + grid_scroll_bar.on("scroll", function (event) { grid_scroll_bar_rows = $(event.currentTarget).closest(".grid-scroll-bar-rows"); - var scroll_left = $(this).scrollLeft(); - // Sync the form grid's left position with the scroll bar form_grid.css("position", "relative"); - form_grid.css("left", -scroll_left + "px"); + form_grid.css("left", -$(this).scrollLeft() + "px"); }); }); } @@ -221,6 +203,8 @@ export default class Grid { this.form_grid.on("touchend", () => { isTouchScrolling = false; }); + + this.handle_scroll_bar(); } this.setup_add_row(); diff --git a/frappe/public/js/frappe/form/grid_row.js b/frappe/public/js/frappe/form/grid_row.js index 7f78ba0827..9e40032db2 100644 --- a/frappe/public/js/frappe/form/grid_row.js +++ b/frappe/public/js/frappe/form/grid_row.js @@ -408,18 +408,14 @@ export default class GridRow { this.columns = {}; this.update_user_settings_for_grid(); this.grid_settings_dialog.hide(); - setTimeout(() => { - this.grid.handle_scroll_bar(); - }, 500); + this.grid.handle_scroll_bar(); }); this.grid_settings_dialog.set_secondary_action_label(__("Reset to default")); this.grid_settings_dialog.set_secondary_action(() => { this.reset_user_settings_for_grid(); this.grid_settings_dialog.hide(); - setTimeout(() => { - this.grid.handle_scroll_bar(); - }, 500); + this.grid.handle_scroll_bar(); }); } From 756670d675a38f585dafc99c0ad17fa950a505e0 Mon Sep 17 00:00:00 2001 From: Ejaaz Khan Date: Mon, 27 Jan 2025 10:57:57 +0530 Subject: [PATCH 4/5] refactor: optimize grid selection by reusing existing reference --- frappe/public/js/frappe/form/grid.js | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index 87035b425f..a834d89bee 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -51,25 +51,19 @@ export default class Grid { } handle_scroll_bar() { frappe.utils.sleep(500).then(() => { - let form_grid = $(`div[data-fieldname="${this.df.fieldname}"] .form-grid`); - let grid_body_width = $( - `div[data-fieldname="${this.df.fieldname}"] .grid-body` - ).width(); + let grid_body_width = this.wrapper.find(".grid-body").width(); - let grid_scroll_bar = $(`div[data-fieldname="${this.df.fieldname}"] .grid-scroll-bar`); - let grid_scroll_bar_rows = $( - `div[data-fieldname="${this.df.fieldname}"] .grid-scroll-bar-rows` - ); + let grid_scroll_bar = this.wrapper.find(".grid-scroll-bar"); - grid_scroll_bar.width(form_grid.width()); + let grid_scroll_bar_rows = this.wrapper.find(".grid-scroll-bar-rows"); + + grid_scroll_bar.width(this.form_grid.width()); grid_scroll_bar_rows.width(grid_body_width); - grid_scroll_bar.on("scroll", function (event) { - grid_scroll_bar_rows = $(event.currentTarget).closest(".grid-scroll-bar-rows"); - + grid_scroll_bar.on("scroll", (event) => { // Sync the form grid's left position with the scroll bar - form_grid.css("position", "relative"); - form_grid.css("left", -$(this).scrollLeft() + "px"); + this.form_grid.css("position", "relative"); + this.form_grid.css("left", -$(event.currentTarget).scrollLeft() + "px"); }); }); } From b12e42f4066dfb10d70fa7173108adf8ef19c699 Mon Sep 17 00:00:00 2001 From: Ejaaz Khan Date: Mon, 27 Jan 2025 11:56:49 +0530 Subject: [PATCH 5/5] fix: add missing px in style --- frappe/public/scss/common/grid.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/scss/common/grid.scss b/frappe/public/scss/common/grid.scss index c58831a076..534d168742 100644 --- a/frappe/public/scss/common/grid.scss +++ b/frappe/public/scss/common/grid.scss @@ -625,7 +625,7 @@ } .grid-static-col.col-xs-4 { flex: 1 0 200px; - width: 200; + width: 200px; } .grid-static-col.col-xs-5 { flex: 1 0 250px;