Skip to content

fixed multiple tabs performance - #2357

Open
Yago004 wants to merge 4 commits into
UPC:mainfrom
Yago004:actions_tab
Open

fixed multiple tabs performance#2357
Yago004 wants to merge 4 commits into
UPC:mainfrom
Yago004:actions_tab

Conversation

@Yago004

@Yago004 Yago004 commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Fixed tabs performance inside machine settings and now hide and show as expected.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Machine Settings left-hand tab navigation to rely more on Angular runtime state (showmachine.*) for showing/hiding certain tabs (notably base/active-dependent tabs), aiming to improve perceived performance and make tab visibility update without a full page reload.

Changes:

  • Moved “Actions” and “Screenshot” base-machine gating from server-side $domain->is_base checks to Angular (ng-hide="showmachine.is_base").
  • Updated “Clones” and “System overview” tabs to be controlled by Angular runtime state (showmachine.is_base, showmachine.has_clones, showmachine.is_active).
  • Relaxed some server-side conditions (e.g., removed $domain->is_active / $domain->is_base checks in the tab header) so the tab header can react to state changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +2 to +3
% if (($USER->can_change_settings($domain->id))) {
<a class="nav-link <%= $active_actions %>" id="v-pills-actions-tab" ng-hide="showmachine.is_base" href="#v-pills-actions" data-toggle="pill" role="tab" aria-controls="v-pills-actions" aria-selected="true" ng-disabled="edit"><%=l 'Actions' %></a>
Comment on lines +17 to +18
% if ($USER->can_change_settings($domain->id)) {
<a class="nav-link" id="v-pills-screenshot-tab" ng-hide="showmachine.is_base" href="#v-pills-screenshot" data-toggle="pill" role="tab" aria-controls="v-pills-screenshot" aria-selected="false"><%=l 'Screenshot' %></a>
Comment on lines +35 to +36
% if ($USER->is_admin()){
<a class="nav-link" id="v-pills-clones-tab" ng-show="showmachine.has_clones && showmachine.is_base" href="#v-pills-clones" data-toggle="pill" role="tab" aria-controls="v-pills-clones" aria-selected="false"><%=l 'Clones' %></a>
Comment on lines +46 to +47
% if ( $monitoring && $USER->is_admin()) {
<a class="nav-link" id="v-pills-monitoring-tab" ng-show="showmachine.is_active" href="#v-pills-monitoring" data-toggle="pill" role="tab" aria-controls="v-pills-monitoring" aria-selected="false"><%=l 'System overview' %></a>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

templates/main/settings_machine_tabs_head.html.ep:3

  • Hiding the Actions tab/pane via ng-hide can leave the UI in a state where the currently active tab becomes hidden (e.g., user is on Actions, then the machine is converted to base via “Prepare Base”), resulting in an empty content area with no visible active tab. This needs a fallback that switches to a visible tab when a tab becomes hidden (same pattern also affects Screenshot/Clones/Monitoring).
        <a class="nav-link <%= $active_actions %>" id="v-pills-actions-tab" ng-hide="showmachine.is_base" href="#v-pills-actions" data-toggle="pill" role="tab" aria-controls="v-pills-actions" aria-selected="true" ng-disabled="edit"><%=l 'Actions' %></a>

templates/main/settings_machine_tabs_head.html.ep:47

  • The Monitoring tab link is now gated only by showmachine.is_active, but the corresponding tab pane is still server-gated by $domain->internal_id in the body template. If internal_id is false, the link can become visible (when the VM starts) but the pane will never exist, leading to a broken tab target.
    % if ( $monitoring && $USER->is_admin()) {
        <a class="nav-link" id="v-pills-monitoring-tab" ng-show="showmachine.is_active" href="#v-pills-monitoring" data-toggle="pill" role="tab" aria-controls="v-pills-monitoring" aria-selected="false"><%=l 'System overview' %></a>

% if (($USER->can_change_settings($domain->id)) && !$domain->is_base) {
<a class="nav-link <%= $active_actions %>" id="v-pills-actions-tab" href="#v-pills-actions" data-toggle="pill" role="tab" aria-controls="v-pills-actions" aria-selected="true" ng-disabled="edit"><%=l 'Actions' %></a>
% if (($USER->can_change_settings($domain->id))) {
<a class="nav-link <%= $active_actions %>" id="v-pills-actions-tab" ng-hide="showmachine.is_base" href="#v-pills-actions" data-toggle="pill" role="tab" aria-controls="v-pills-actions" aria-selected="true" ng-disabled="edit"><%=l 'Actions' %></a>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (5)

templates/main/settings_machine_tabs_head.html.ep:18

  • Same issue as Actions: ng-hide="showmachine.is_base" makes the Screenshot tab visible until showmachine is populated. Guard the expression (or use ng-if) so base machines don’t briefly show this tab during initial load.
    %   if ($USER->can_change_settings($domain->id)) {
        <a class="nav-link" id="v-pills-screenshot-tab" ng-hide="showmachine.is_base" href="#v-pills-screenshot" data-toggle="pill" role="tab" aria-controls="v-pills-screenshot" aria-selected="false"><%=l 'Screenshot' %></a>

templates/main/settings_machine_tabs_body.html.ep:38

  • Same concern as the Screenshot pane: ng-hide="showmachine.is_base" keeps the Actions DOM (and watchers) around even when hidden, and it will be visible until showmachine is loaded because showmachine.is_base starts undefined. Switching to ng-if="showmachine && !showmachine.is_base" avoids the initial flash and reduces overhead when Actions shouldn’t be available.
% if ($USER->can_change_settings($domain->id)) {
    <div class="tab-pane fade <%= $show_actions %>" id="v-pills-actions" role="tabpanel" aria-labelledby="v-pills-actions-tab" ng-hide="showmachine.is_base">
        %= include 'main/vm_actions'

templates/main/settings_machine_tabs_head.html.ep:3

  • ng-hide="showmachine.is_base" will evaluate to false when showmachine is still undefined, so the Actions tab can briefly appear (and be focusable) before machine info is loaded. Using ng-if (or guarding with !showmachine || ...) avoids that initial flash and also keeps the tab out of the DOM when it should be hidden.

This issue also appears on line 17 of the same file.

    %   if (($USER->can_change_settings($domain->id))) {
        <a class="nav-link <%= $active_actions %>" id="v-pills-actions-tab" ng-hide="showmachine.is_base" href="#v-pills-actions" data-toggle="pill" role="tab" aria-controls="v-pills-actions" aria-selected="<%= $active_actions ? 'true' : 'false' %>" ng-disabled="edit"><%=l 'Actions' %></a>

templates/main/settings_machine_tabs_body.html.ep:28

  • This tab pane is always rendered server-side and only hidden with ng-hide. That means its full HTML (and Angular watchers) are present even for base machines, and it can also briefly render before showmachine is loaded (because showmachine.is_base is initially undefined). Using ng-if="showmachine && !showmachine.is_base" prevents both the initial flash and unnecessary DOM/watchers when the tab should be hidden.

This issue also appears on line 36 of the same file.

% if ($USER->can_change_settings($domain->id)) {
    <div class="tab-pane fade" id="v-pills-screenshot" role="tabpanel" aria-labelledby="v-pills-screenshot-tab" ng-hide="showmachine.is_base">
        %= include 'main/vm_screenshot'

templates/main/settings_machine_tabs_body.html.ep:66

  • The monitoring content includes multiple data-netdata elements. With ng-show, those elements remain in the DOM even when the VM is inactive, so any netdata auto-discovery/initialization code may still process them despite the tab being hidden. Using ng-if="showmachine && showmachine.is_active" ensures the monitoring DOM (and netdata widgets) only exist while the VM is active.
% if ( $monitoring && $USER->is_admin && $domain->internal_id) {
    <div class="tab-pane fade" id="v-pills-monitoring" role="tabpanel" aria-labelledby="v-pills-monitoring-tab" ng-show="showmachine.is_active">
        %= include 'main/vm_monitoring'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants