Skip to content

Commit df3bb9e

Browse files
ruchamahabalruchamahabal
committed
perf: Optimise Rendering
- optimise get_children function - use promises instead of callbacks - optimise selectors - use const wherever possible - use pure js instead of jquery for connectors for faster rendering
1 parent 6eec251 commit df3bb9e

3 files changed

Lines changed: 79 additions & 92 deletions

File tree

‎erpnext/hr/page/organizational_chart/organizational_chart.py‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,33 +2,25 @@
22
import frappe
33

44
@frappe.whitelist()
5-
def get_children(parent=None, company=None, exclude_node=None, is_root=False, is_tree=False, fields=None):
5+
def get_children(parent=None, company=None):
66

77
filters = [['status', '!=', 'Left']]
88
if company and company != 'All Companies':
99
filters.append(['company', '=', company])
1010

11-
if not fields:
12-
fields = ['employee_name as name', 'name as id', 'reports_to', 'image', 'designation as title']
13-
14-
if is_root:
15-
parent = ''
16-
17-
if exclude_node:
18-
filters.append(['name', '!=', exclude_node])
19-
2011
if parent and company and parent != company:
2112
filters.append(['reports_to', '=', parent])
2213
else:
2314
filters.append(['reports_to', '=', ''])
2415

25-
employees = frappe.get_list('Employee', fields=fields,
26-
filters=filters, order_by='name')
16+
employees = frappe.get_list('Employee',
17+
fields=['employee_name as name', 'name as id', 'reports_to', 'image', 'designation as title'],
18+
filters=filters,
19+
order_by='name'
20+
)
2721

2822
for employee in employees:
29-
is_expandable = frappe.get_all('Employee', filters=[
30-
['reports_to', '=', employee.get('id')]
31-
])
23+
is_expandable = frappe.db.count('Employee', filters={'reports_to': employee.get('id')})
3224
employee.connections = get_connections(employee.id)
3325
employee.expandable = 1 if is_expandable else 0
3426

‎erpnext/public/js/hierarchy_chart/hierarchy_chart_desktop.js‎

Lines changed: 41 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ erpnext.HierarchyChart = class {
77
- this method should return id, name, title, image, and connections for each node
88
*/
99
constructor(doctype, wrapper, method) {
10-
this.wrapper = $(wrapper);
1110
this.page = wrapper.page;
1211
this.method = method;
1312
this.doctype = doctype;
@@ -61,6 +60,8 @@ erpnext.HierarchyChart = class {
6160
frappe.breadcrumbs.add('HR');
6261

6362
let me = this;
63+
if ($(`[data-fieldname="company"]`).length) return;
64+
6465
let company = this.page.add_field({
6566
fieldtype: 'Link',
6667
options: 'Company',
@@ -131,32 +132,30 @@ erpnext.HierarchyChart = class {
131132
method: me.method,
132133
args: {
133134
company: me.company
134-
},
135-
callback: function(r) {
136-
if (r.message.length) {
137-
let nodes = r.message;
138-
let node = undefined;
139-
let first_root = undefined;
140-
141-
$.each(nodes, (i, data) => {
142-
node = new me.Node({
143-
id: data.id,
144-
parent: $('<li class="child-node"></li>').appendTo(me.$hierarchy.find('.node-children')),
145-
parent_id: undefined,
146-
image: data.image,
147-
name: data.name,
148-
title: data.title,
149-
expandable: true,
150-
connections: data.connections,
151-
is_root: true
152-
});
153-
154-
if (i == 0)
155-
first_root = node;
135+
}
136+
}).then(r => {
137+
if (r.message.length) {
138+
let node = undefined;
139+
let first_root = undefined;
140+
141+
$.each(r.message, (i, data) => {
142+
node = new me.Node({
143+
id: data.id,
144+
parent: $('<li class="child-node"></li>').appendTo(me.$hierarchy.find('.node-children')),
145+
parent_id: undefined,
146+
image: data.image,
147+
name: data.name,
148+
title: data.title,
149+
expandable: true,
150+
connections: data.connections,
151+
is_root: true
156152
});
157153

158-
me.expand_node(first_root);
159-
}
154+
if (i == 0)
155+
first_root = node;
156+
});
157+
158+
me.expand_node(first_root);
160159
}
161160
});
162161
}
@@ -204,18 +203,14 @@ erpnext.HierarchyChart = class {
204203
}
205204

206205
get_child_nodes(node_id) {
207-
let me = this;
208206
return new Promise(resolve => {
209207
frappe.call({
210208
method: this.method,
211209
args: {
212210
parent: node_id,
213-
company: me.company
214-
},
215-
callback: (r) => {
216-
resolve(r.message);
211+
company: this.company
217212
}
218-
});
213+
}).then(r => resolve(r.message));
219214
});
220215
}
221216

@@ -266,27 +261,28 @@ erpnext.HierarchyChart = class {
266261
}
267262

268263
add_connector(parent_id, child_id) {
264+
// using pure javascript for better performance
269265
const parent_node = document.querySelector(`#${parent_id}`);
270266
const child_node = document.querySelector(`#${child_id}`);
271267

272268
let path = document.createElementNS('http://www.w3.org/2000/svg', 'path');
273269

274270
// we need to connect right side of the parent to the left side of the child node
275-
let pos_parent_right = {
271+
const pos_parent_right = {
276272
x: parent_node.offsetLeft + parent_node.offsetWidth,
277273
y: parent_node.offsetTop + parent_node.offsetHeight / 2
278274
};
279-
let pos_child_left = {
275+
const pos_child_left = {
280276
x: child_node.offsetLeft - 5,
281277
y: child_node.offsetTop + child_node.offsetHeight / 2
282278
};
283279

284-
let connector = this.get_connector(pos_parent_right, pos_child_left);
280+
const connector = this.get_connector(pos_parent_right, pos_child_left);
285281

286282
path.setAttribute('d', connector);
287283
this.set_path_attributes(path, parent_id, child_id);
288284

289-
$('#connectors').append(path);
285+
document.getElementById('connectors').appendChild(path);
290286
}
291287

292288
get_connector(pos_parent_right, pos_child_left) {
@@ -330,21 +326,23 @@ erpnext.HierarchyChart = class {
330326
set_path_attributes(path, parent_id, child_id) {
331327
path.setAttribute("data-parent", parent_id);
332328
path.setAttribute("data-child", child_id);
329+
const parent = $(`#${parent_id}`);
333330

334-
if ($(`#${parent_id}`).hasClass('active')) {
331+
if (parent.hasClass('active')) {
335332
path.setAttribute("class", "active-connector");
336333
path.setAttribute("marker-start", "url(#arrowstart-active)");
337334
path.setAttribute("marker-end", "url(#arrowhead-active)");
338-
} else if ($(`#${parent_id}`).hasClass('active-path')) {
335+
} else if (parent.hasClass('active-path')) {
339336
path.setAttribute("class", "collapsed-connector");
340337
path.setAttribute("marker-start", "url(#arrowstart-collapsed)");
341338
path.setAttribute("marker-end", "url(#arrowhead-collapsed)");
342339
}
343340
}
344341

345342
set_selected_node(node) {
346-
// remove .active class from the current node
347-
$('.active').removeClass('active');
343+
// remove active class from the current node
344+
if (this.selected_node)
345+
this.selected_node.$link.removeClass('active');
348346

349347
// add active class to the newly selected node
350348
this.selected_node = node;
@@ -411,9 +409,9 @@ erpnext.HierarchyChart = class {
411409
}
412410

413411
remove_levels_after_node(node) {
414-
let level = $(`#${node.id}`).parent().parent().parent();
412+
let level = $(`#${node.id}`).parent().parent().parent().index();
415413

416-
level = $('.hierarchy > li:eq('+ level.index() + ')');
414+
level = $('.hierarchy > li:eq('+ level + ')');
417415
level.nextAll('li').remove();
418416

419417
let nodes = level.find('.node-card');
@@ -431,8 +429,8 @@ erpnext.HierarchyChart = class {
431429
remove_orphaned_connectors() {
432430
let paths = $('#connectors > path');
433431
$.each(paths, (_i, path) => {
434-
let parent = $(path).data('parent');
435-
let child = $(path).data('child');
432+
const parent = $(path).data('parent');
433+
const child = $(path).data('child');
436434

437435
if ($(`#${parent}`).length && $(`#${child}`).length)
438436
return;

‎erpnext/public/js/hierarchy_chart/hierarchy_chart_mobile.js‎

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ erpnext.HierarchyChartMobile = class {
77
- this method should return id, name, title, image, and connections for each node
88
*/
99
constructor(doctype, wrapper, method) {
10-
this.wrapper = $(wrapper);
1110
this.page = wrapper.page;
1211
this.method = method;
1312
this.doctype = doctype;
@@ -63,6 +62,8 @@ erpnext.HierarchyChartMobile = class {
6362
frappe.breadcrumbs.add('HR');
6463

6564
let me = this;
65+
if ($(`[data-fieldname="company"]`).length) return;
66+
6667
let company = this.page.add_field({
6768
fieldtype: 'Link',
6869
options: 'Company',
@@ -139,24 +140,21 @@ erpnext.HierarchyChartMobile = class {
139140
args: {
140141
company: me.company
141142
},
142-
callback: function(r) {
143-
if (r.message.length) {
144-
let nodes = r.message;
145-
146-
$.each(nodes, (_i, data) => {
147-
return new me.Node({
148-
id: data.id,
149-
parent: me.$hierarchy.find('.root-level'),
150-
parent_id: undefined,
151-
image: data.image,
152-
name: data.name,
153-
title: data.title,
154-
expandable: true,
155-
connections: data.connections,
156-
is_root: true
157-
});
143+
}).then(r => {
144+
if (r.message.length) {
145+
$.each(r.message, (_i, data) => {
146+
return new me.Node({
147+
id: data.id,
148+
parent: me.$hierarchy.find('.root-level'),
149+
parent_id: undefined,
150+
image: data.image,
151+
name: data.name,
152+
title: data.title,
153+
expandable: true,
154+
connections: data.connections,
155+
is_root: true
158156
});
159-
}
157+
});
160158
}
161159
});
162160
}
@@ -237,11 +235,8 @@ erpnext.HierarchyChartMobile = class {
237235
parent: node_id,
238236
company: me.company,
239237
exclude_node: exclude_node
240-
},
241-
callback: (r) => {
242-
resolve(r.message);
243238
}
244-
});
239+
}).then(r => resolve(r.message));
245240
});
246241
}
247242

@@ -286,10 +281,10 @@ erpnext.HierarchyChartMobile = class {
286281
}
287282

288283
add_connector(parent_id, child_id) {
289-
let parent_node = document.querySelector(`#${parent_id}`);
290-
let child_node = document.querySelector(`#${child_id}`);
284+
const parent_node = document.querySelector(`#${parent_id}`);
285+
const child_node = document.querySelector(`#${child_id}`);
291286

292-
let path = document.createElementNS('http://www.w3.org/2000/svg', 'path');
287+
const path = document.createElementNS('http://www.w3.org/2000/svg', 'path');
293288

294289
let connector = undefined;
295290

@@ -299,10 +294,10 @@ erpnext.HierarchyChartMobile = class {
299294
connector = this.get_connector_for_collapsed_node(parent_node, child_node);
300295
}
301296

302-
path.setAttribute("d", connector);
297+
path.setAttribute('d', connector);
303298
this.set_path_attributes(path, parent_id, child_id);
304299

305-
$('#connectors').append(path);
300+
document.getElementById('connectors').appendChild(path);
306301
}
307302

308303
get_connector_for_active_node(parent_node, child_node) {
@@ -351,19 +346,21 @@ erpnext.HierarchyChartMobile = class {
351346
set_path_attributes(path, parent_id, child_id) {
352347
path.setAttribute("data-parent", parent_id);
353348
path.setAttribute("data-child", child_id);
349+
const parent = $(`#${parent_id}`);
354350

355-
if ($(`#${parent_id}`).hasClass('active')) {
351+
if (parent.hasClass('active')) {
356352
path.setAttribute("class", "active-connector");
357353
path.setAttribute("marker-start", "url(#arrowstart-active)");
358354
path.setAttribute("marker-end", "url(#arrowhead-active)");
359-
} else if ($(`#${parent_id}`).hasClass('active-path')) {
355+
} else if (parent.hasClass('active-path')) {
360356
path.setAttribute("class", "collapsed-connector");
361357
}
362358
}
363359

364360
set_selected_node(node) {
365361
// remove .active class from the current node
366-
$('.active').removeClass('active');
362+
if (this.selected_node)
363+
this.selected_node.$link.removeClass('active');
367364

368365
// add active class to the newly selected node
369366
this.selected_node = node;
@@ -494,9 +491,9 @@ erpnext.HierarchyChartMobile = class {
494491
}
495492

496493
remove_levels_after_node(node) {
497-
let level = $(`#${node.id}`).parent().parent();
494+
let level = $(`#${node.id}`).parent().parent().index();
498495

499-
level = $('.hierarchy-mobile > li:eq('+ (level.index()) + ')');
496+
level = $('.hierarchy-mobile > li:eq('+ level + ')');
500497
level.nextAll('li').remove();
501498

502499
let current_node = level.find(`#${node.id}`);
@@ -512,8 +509,8 @@ erpnext.HierarchyChartMobile = class {
512509
remove_orphaned_connectors() {
513510
let paths = $('#connectors > path');
514511
$.each(paths, (_i, path) => {
515-
let parent = $(path).data('parent');
516-
let child = $(path).data('child');
512+
const parent = $(path).data('parent');
513+
const child = $(path).data('child');
517514

518515
if ($(`#${parent}`).length && $(`#${child}`).length)
519516
return;

0 commit comments

Comments
 (0)
⚡