[DREAM-729] Fix lazy loaded Action menu positioning#478
Conversation
🦋 Changeset detectedLatest commit: 4713378 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
dombesz
changed the title
dombesz
force-pushed
the
bug/dream-729-lazy-loaded-action-menu-positioning-is-incorrect-when-opened-at-the-bottom-of-the-page
branch
from
34bd503 to
31627a3
Compare
…ned at the bottom of the page. https://community.openproject.org/wp/DREAM-729
dombesz
force-pushed
the
bug/dream-729-lazy-loaded-action-menu-positioning-is-incorrect-when-opened-at-the-bottom-of-the-page
branch
from
90c2e20 to
da4d2ca
Compare
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a positioning issue in the Alpha ActionMenu when its menu items are lazy-loaded via include-fragment: if the menu is opened near the bottom of the viewport, the overlay now recalculates its position after the async content is inserted.
Changes:
- Trigger an overlay reposition (
overlay.update()) afterinclude-fragmentcontent replacement. - Add a system test asserting the deferred-loaded overlay stays within the viewport when invoked at the bottom of the page.
- Add a patch changeset entry for the behavior fix.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
app/components/primer/alpha/action_menu/action_menu_element.ts |
Updates overlay positioning after deferred content loads by calling overlay.update() on fragment replacement. |
test/system/alpha/action_menu_test.rb |
Adds a system regression test for deferred-content overlay positioning near the viewport bottom. |
.changeset/silly-lights-fall.md |
Records a patch-level change for the ActionMenu positioning fix. |
| overlay_in_viewport = page.evaluate_script(<<~JS) | ||
| (function() { | ||
| var el = document.querySelector('anchored-position') | ||
| var rect = el.getBoundingClientRect() | ||
| return rect.bottom <= (window.innerHeight || document.documentElement.clientHeight) | ||
| })() | ||
| JS |
| '@openproject/primer-view-components': patch | ||
| --- | ||
|
|
||
| Fix lazy loaded action-menu positioning when opened at the bottom of the page. |
myabc
changed the title
|
@dombesz this fix might be related, albeit for SelectPanels: primer#4092 |
HDinger
left a comment
HDinger
left a comment
There was a problem hiding this comment.
As written in the ticket, I cannot reproduce so I don't know wether this actually fixes it 🤷 I do have a remark regarding the test though..
| })() | ||
| JS | ||
|
|
||
| assert overlay_in_viewport, "Overlay should be positioned within the viewport after deferred content loads" |
There was a problem hiding this comment.
This test is somehow cumbersome.. You can also create a (hidden) preview where you reproduce that scenario and test it without all of that JS.
Ticket
https://community.openproject.org/wp/DREAM-729
What are you trying to accomplish?
When an ActionMenu with lazy-loaded content (src=) is opened near the bottom of the viewport, the overlay is not repositioning itself after the async menu finished loading. The overlay calculated its position on open (before the content existed), and never updated once the
include-fragmentreplaced the placeholder with the menu response.Screenshots
Before
After
Closes # (type the GitHub issue number after #)
Risk Assessment
What approach did you choose and why?
The fix is to call
overlay.update()inside#handleIncludeFragmentReplaced()so theanchored-positionelement position recalculates after the lazily loaded fragment is displayed.Merge checklist