Skip to content

NAS-143761 / 26.0.0-RC.1 / Keep the Hosts dialog usable when editing or adding a host (by AlexKarpov98) - #14134

Merged
aervin merged 2 commits into
release/26.0.0-RC.1from
NAS-143761-26.0.0-RC.1
Sep 21, 2026
Merged

aervin merged 2 commits into
release/26.0.0-RC.1from
NAS-143761-26.0.0-RC.1

Conversation

@bugclerk

Copy link
Copy Markdown
Contributor

Preview:

Screen.Recording.2026-09-15.at.13.35.05.mov

Change

Editing or adding a host from the NVMe-oF Hosts management dialog dropped the user back on the
Subsystems screen instead of returning them to the list.

The reported cause was already gone. In 26.0.0-RC.1 ManageHostsDialog.onAdd()/onEdit() called
this.dialogRef.close() before opening the slide-in — with a comment saying it was to stop the
dialog appearing behind the form. ManagePortsDialog never did this, which is exactly why Ports
behaved correctly. That call was removed in 08da727 when the slide-in became
FormSidePanelService.

What is still broken — and for both dialogs — is the layer underneath. That migration came with
the comment "the side panel … paints on top of this dialog's backdrop", but the premise was
already false: three weeks earlier e69bcee had added
.tn-side-panel__overlay { z-index: 999 !important } to _tn-styles.scss, dropping the panel below
the CDK overlay layer (1000). tn-side-panel portals its overlay to document.body, and
.cdk-overlay-container is a body sibling — so Edit / Add New opened the host form behind the
dialog's dark backdrop: invisible and click-blocked.

The container is a single element with a single z-index, so no one value can order the
panel correctly in both directions at once:

at z-index: 1000 at z-index: 999 (before this PR)
Panel opened from an open dialog panel on top ✅ panel buried under the dialog ❌
Dropdown / confirm dialog opened from the panel hidden behind the panel ❌ on top ✅

So FormSidePanelContainerComponent now re-homes its portaled overlay into
OverlayContainer.getContainerElement(), where panel and CDK panes are siblings at the same
z-index and DOM order — i.e. open order — arbitrates. Both columns above become ✅.

  • Scheduled with afterNextRender({ read: … }): the library's own body-append runs in the default
    mixedReadWrite phase, so the read phase is guaranteed to follow it in the same render cycle.
  • The overlay is located via the data-tn-panel id the library puts on both the tn-side-panel
    host and the portaled overlay — the correlation it documents for exactly this.
  • _tn-styles.scss keeps the 999 rule for the ~6 <tn-side-panel>s declared in templates
    (dataset-form, zvol-form, vm-wizard, …), plus a scoped rule restoring 1000 inside the container.

Two notes on scope: this fixes Ports identically, since it carries the same latent problem; and
the only other dialog that opens a side panel, session-expiring-dialog, closes itself first and
was never affected.

Testing

The appliance was unreachable, so the stacking was verified in a real browser against a synthetic
page built from the verbatim CDK and tn-side-panel CSS, probing with elementFromPoint:

  • before — top element at the panel's centre is cdk-overlay-backdrop, panel not hit-testable;

  • after — top element is tn-side-panel__panel, and a CDK pane appended later still sits above
    it.

  • yarn test src/app/modules/slide-ins → 36 passing, src/app/pages/sharing/nvme-of → 155 passing

  • tsc -p src/tsconfig.app.json clean; eslint and stylelint clean

  • New spec in form-side-panel.service.spec.ts asserts the overlay lands in the CDK container
    after a pane that was already open — confirmed to fail when the fix is reverted

  • expect(DialogRef.close).not.toHaveBeenCalled() added to the manage-hosts Add/Edit tests, pinning
    the originally reported regression directly

Heads-up for reviewers: node_modules had @truenas/ui-components at 0.1.57 against ~0.7.6 in
package.json; run yarn install before the specs or every tn-* spec fails.

Original PR: #14131

@bugclerk

Copy link
Copy Markdown
Contributor Author

Automatic cherry-pick failed. Please resolve conflicts by running:

git reset --hard HEAD~1
git cherry-pick -x 0a5ce70304f72741d1d2906a9e846d97fb2b8cf5
git cherry-pick -x 1a3747023b8becf707d9fa1fe7b1c3ec3a82f1b3
git cherry-pick -x 9614fe6d6667b3e7758d28c446437786583980fd
git cherry-pick -x 2c0b892567b8ed86aea141594472c058e9b8a183

If the original PR was merged via a squash, you can just cherry-pick the squashed commit:

git reset --hard HEAD~1
git cherry-pick -x 17762cea615ee3c1e5a59bc06235f83c544bd400

@bugclerk

Copy link
Copy Markdown
Contributor Author

…a host

Backport of #14131 to 26.0.0-RC.1, where the side-panel forms it was
written against do not exist. The dialog closed itself before opening
the slide-in, so closing the form landed the user back on the subsystem
instead of the host list. The slide-in is a CDK overlay created after
the dialog's, so it already paints above it — as the Manage Ports dialog
has always relied on. Drops the form-side-panel and tn-side-panel
stacking changes, which have no counterpart on this branch.

Claude-Session: https://claude.ai/code/session_01YWb7ZvZPxunLPDKRaBnce9
@codecov

codecov Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (release/26.0.0-RC.1@944a225). Learn more about missing BASE report.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@                  Coverage Diff                   @@
##             release/26.0.0-RC.1   #14134   +/-   ##
======================================================
  Coverage                       ?   87.61%           
======================================================
  Files                          ?     1930           
  Lines                          ?    75206           
  Branches                       ?     8939           
======================================================
  Hits                           ?    65894           
  Misses                         ?     9312           
  Partials                       ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AlexKarpov98 AlexKarpov98 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.

ported. @aervin

@AlexKarpov98
AlexKarpov98 marked this pull request as ready for review September 18, 2026 12:43
@AlexKarpov98
AlexKarpov98 requested a review from a team as a code owner September 18, 2026 12:44
@AlexKarpov98
AlexKarpov98 requested review from aervin and removed request for a team September 18, 2026 12:44
@aervin
aervin merged commit 55dc1cb into release/26.0.0-RC.1 Sep 21, 2026
13 checks passed
@aervin
aervin deleted the NAS-143761-26.0.0-RC.1 branch September 21, 2026 14:38
@bugclerk

Copy link
Copy Markdown
Contributor Author

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Sep 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants