Skip to content

refactor(nav-drawer): Use native dialog for non-relative nav-drawers#2194

Open
rkaraivanov wants to merge 4 commits intomasterfrom
rkaraivanov/nav-drawer-to-popover
Open

refactor(nav-drawer): Use native dialog for non-relative nav-drawers#2194
rkaraivanov wants to merge 4 commits intomasterfrom
rkaraivanov/nav-drawer-to-popover

Conversation

@rkaraivanov
Copy link
Copy Markdown
Member

Description

  • Use native dialog for non-relative drawers.
  • Expose closing events when a user closes the drawer through interaction.

Type of Change

  • New feature (non-breaking change that adds functionality)
  • Refactoring (code improvements without functional changes)

Related Issues

Closes #2122, #2186, #2187

Checklist

  • My code follows the project's coding standards
  • I have tested my changes locally
  • I have updated documentation if needed

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors igc-nav-drawer to use the native <dialog> element for non-relative drawers (top-layer rendering) and adds user-interaction close events, aligning with the goal of avoiding z-index/stacking issues and enabling external consumers to react to user-driven closes.

Changes:

  • Switch non-relative nav-drawer rendering/behavior to a native modal <dialog> and add keepOpenOnEscape.
  • Emit igcClosing (cancelable) and igcClosed for user-interaction closes; export the component event map type.
  • Update Storybook and unit tests to cover dialog-based behavior and the new close events.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
stories/nav-drawer.stories.ts Adds Storybook action handlers and a keepOpenOnEscape control; updates drawer item content markup.
src/index.ts Exports IgcNavDrawerComponentEventMap from the package entrypoint.
src/components/nav-drawer/nav-drawer.ts Refactors non-relative mode to native <dialog>, adds close events + keepOpenOnEscape, and adjusts rendering logic.
src/components/nav-drawer/nav-drawer.spec.ts Updates a11y/DOM tests and adds behavior/event coverage for the native dialog implementation.

Comment thread src/components/nav-drawer/nav-drawer.ts
Comment on lines +127 to +133
private _handleOpenState(): void {
if (this._isRelative) {
return;
}

this.open ? this._dialog?.showModal() : this._dialog?.close();
}
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

_handleOpenState() calls dialog.close() immediately when open becomes false. Because close() hides the <dialog> (removes it from the top layer), the existing CSS slide-out/opacity transitions on [part='base'] (driven by :host([open])) can’t run on close anymore, which is likely a behavioral regression. To preserve the drawer’s closing transition, keep the dialog open while you toggle the host open attribute, then close the native dialog only after the transition finishes (or replace the CSS transitions with an explicit animation that works with dialog close).

Copilot uses AI. Check for mistakes.
Comment thread src/components/nav-drawer/nav-drawer.spec.ts Outdated
Comment thread stories/nav-drawer.stories.ts Outdated
Comment thread stories/nav-drawer.stories.ts Outdated
- Render correctly on first update with initial open state
- Correctly sync native dialog open state when changing drawer positioning
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request nav-drawer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Navigation Drawer] Use the Popover/Dialog API as a basis for the component

2 participants