Skip to content

fix(popover): portal into closest dialog to stay on top layer#300

Merged
desmondinho merged 1 commit intomainfrom
fix/popovers-in-modals
Mar 19, 2026
Merged

fix(popover): portal into closest dialog to stay on top layer#300
desmondinho merged 1 commit intomainfrom
fix/popovers-in-modals

Conversation

@desmondinho
Copy link
Copy Markdown
Contributor

@desmondinho desmondinho commented Mar 19, 2026

Description

Closes #299

Popovers (used by LumexSelect, etc.) were portaled to document.body, which sits below the native <dialog> top layer. This made select dropdowns invisible when rendered inside a LumexModal.

What's been done?

  • Detect if the popover originates from within a <dialog> element and portal into that dialog instead, keeping the popover within the top layer stacking context.

Checklist

  • My code follows the project's coding style and guidelines.
  • I have included inline docs for my changes, where applicable.
  • I have added, updated or removed tests according to my changes.
  • All tests are passing.
  • There's an open issue for the PR that I am making.

Additional Notes

Summary by CodeRabbit

  • Bug Fixes
    • Improved popover rendering when used within dialog elements, with more reliable portal destination resolution and error handling.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 19, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The portal utility function signature and resolution logic were updated to accept either an HTMLElement or selector string as a destination parameter. Popover initialization was modified to detect the nearest ancestor <dialog> element and pass it as the destination to portal calls, ensuring proper layering within modal contexts.

Changes

Cohort / File(s) Summary
Popover initialization
src/LumexUI/wwwroot/js/components/popover.js, src/LumexUI/wwwroot/js/components/popover.bundle.js
Modified to determine the closest ancestor dialog element and pass it to portal calls for proper modal context handling.
Portal utility refactoring
src/LumexUI/wwwroot/js/utils/dom.js
Updated portal() function to accept destination parameter that resolves from HTMLElement, selector string, or defaults to document.body. Changed error message to generic fallback.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A popover no more hides,
Behind the modal's shadowed sides—
With dialogs as guides so true,
Portal destinations shine through!
~ Hop, hop, hooray! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: making popovers portal into the closest dialog to maintain top-layer positioning.
Description check ✅ Passed The pull request description covers the issue being fixed (#299), explains the problem and solution, lists changes made, and confirms completion of all checklist items.
Linked Issues check ✅ Passed The changes directly address issue #299 by detecting popovers within dialog elements and portaling them into the dialog instead of document.body, making dropdowns visible within modals.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the popover portal destination logic and are directly related to the linked issue; no extraneous modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/popovers-in-modals
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can customize the high-level summary generated by CodeRabbit.

Configure the reviews.high_level_summary_instructions setting to provide custom instructions for generating the high-level summary.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.02%. Comparing base (19b89dc) to head (95ead57).
⚠️ Report is 171 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #300      +/-   ##
==========================================
- Coverage   96.95%   93.02%   -3.93%     
==========================================
  Files          70      166      +96     
  Lines        1542     2750    +1208     
  Branches      150      403     +253     
==========================================
+ Hits         1495     2558    +1063     
- Misses         28      101      +73     
- Partials       19       91      +72     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@desmondinho desmondinho merged commit 05a57f0 into main Mar 19, 2026
3 of 4 checks passed
@desmondinho desmondinho deleted the fix/popovers-in-modals branch March 19, 2026 17:47
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.

Select dropdown appears behind modal and is not interactable

1 participant