fix(organizational-chart): show empty state when no data is available#4024
fix(organizational-chart): show empty state when no data is available#4024krishna-254 wants to merge 2 commits intofrappe:developfrom
Conversation
WalkthroughThe changes remove centralized page styling and inline it within render logic for both desktop and mobile hierarchy chart modules, delete the 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hrms/public/js/hierarchy_chart/hierarchy_chart_desktop.js (1)
126-158: Fix runtime error:setup_page_style()no longer exists.
Line 157 calls a method that isn’t defined in this class anymore, so exporting will throw and the layout won’t be restored. Replace it with an inline restore or reintroduce a valid helper.🐛 Proposed fix (inline restore after export)
- this.setup_page_style(); + // Restore layout after export (setup_page_style was removed) + const hasEmptyState = !!document.getElementById("hierarchy-empty-root"); + this.page.main.css({ + "min-height": hasEmptyState ? "100%" : "300px", + "max-height": hasEmptyState ? "100%" : "700px", + overflow: hasEmptyState ? "" : "auto", + position: hasEmptyState ? "" : "relative", + left: "", + top: "", + });
🤖 Fix all issues with AI agents
In `@hrms/public/js/hierarchy_chart/hierarchy_chart_desktop.js`:
- Around line 259-263: The empty-state template render uses a hard-coded doctype
for permission check; update the can_create call in the hierarchy_empty_state
render so it uses the instance doctype (me.doctype) instead of the literal
"Employee" — locate the render_template call that builds empty_html
(hierarchy_empty_state) and replace frappe.model.can_create("Employee") with
frappe.model.can_create(me.doctype) so CTA visibility honors the class
parameterization.
94b9a0e to
d1ba2d6
Compare
|
This pull request is being marked as inactive because of no recent activity. It will be closed in 3 days if no further activity occurs. |
|
This pull request is being marked as inactive because of no recent activity. It will be closed in 3 days if no further activity occurs. |
|
This pull request is being marked as inactive because of no recent activity. It will be closed in 3 days if no further activity occurs. |
asmitahase
left a comment
There was a problem hiding this comment.
Think simple, much simpler.
| } else { | ||
| me.page.body.find("#hierarchy-empty-root").remove(); | ||
| const empty_html = frappe.render_template("hierarchy_empty_state", { | ||
| doctype: me.doctype, | ||
| company: me.company, | ||
| can_create: frappe.model.can_create(me.doctype), | ||
| device_type: "desktop", | ||
| }); | ||
|
|
||
| $("#hierarchy-chart-wrapper").remove(); | ||
| me.page.body.append(empty_html); | ||
|
|
||
| me.page.main.css({ | ||
| "min-height": "100%", | ||
| "max-height": "100%", | ||
| }); | ||
|
|
||
| (function () { | ||
| const root = document.getElementById("hierarchy-empty-root"); | ||
| if (!root) return; | ||
| try { | ||
| const rect = root.getBoundingClientRect(); | ||
| const windowHeight = window.innerHeight; | ||
| const height = Math.max(300, Math.floor(windowHeight - rect.top - 20)); | ||
| root.style.height = height + "px"; | ||
| root.style.overflow = "auto"; | ||
| root.style.position = "relative"; | ||
| } catch (e) {} | ||
| })(); | ||
| me.page.body.find("#add-doc-btn").on("click", () => { | ||
| frappe.route_options = { company: me.company }; | ||
| frappe.new_doc(me.doctype); | ||
| }); |
There was a problem hiding this comment.
All these calculations aren't required here.
Just do a count call at the beginning in organization_chart.js and render the empty state html there if the employee count is 0.
There was a problem hiding this comment.
This will need setting up the company filter outside the chart object.
| <div class=""> | ||
| <div class="empty-apps-state"> | ||
| <svg class="icon icon-xl" style="stroke: var(--text-light);"> | ||
| <use href="#icon-small-file"></use> |
There was a problem hiding this comment.
Or pass a template variable to reuse this someplace else
| <use href="#icon-small-file"></use> | ||
| </svg> | ||
| <div class="mt-4 mb-2"> | ||
| {{ __("You haven't created a {0} for {1} yet.", [__(doctype), __(company)]) }} |
There was a problem hiding this comment.
Keep user messages short and human, something like "You haven't created any Employee for <company_name>"
| class="btn btn-default btn-sm btn-new-doc hidden-xs" | ||
| id="add-doc-btn" | ||
| > | ||
| {{ __("Create a new {0}", [__(doctype)]) }} |
There was a problem hiding this comment.
Keep the labels short too, "Create New" or simply "Add Employee" should suffice
What does this PR do?
Adds a proper empty state to the Organizational Chart when no employees or reporting relationships are available.
Fixes #3940
What problem does it solve?
Prevents a blank or broken-looking Organizational Chart when data is missing
Screenshots


Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.