Skip to content

Refactor pureCalculateAnchoredPosition to eliminate DOM dependency#683

Merged
francinelucca merged 2 commits intochore/getanchoredposition-error-repofrom
copilot/sub-pr-681
Feb 2, 2026
Merged

Refactor pureCalculateAnchoredPosition to eliminate DOM dependency#683
francinelucca merged 2 commits intochore/getanchoredposition-error-repofrom
copilot/sub-pr-681

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 2, 2026

The pureCalculateAnchoredPosition function claimed to be pure with no DOM dependencies, but was directly accessing window.innerWidth and window.innerHeight when displayInVisibleViewport was enabled.

Changes:

  • Add visibleViewportSize: Size parameter to pureCalculateAnchoredPosition
  • Move window.innerWidth/window.innerHeight reads to caller (getAnchoredPosition)
  • Pass viewport dimensions as argument instead of reading from global
// Before: reads from window global
function pureCalculateAnchoredPosition(..., settings: PositionSettings) {
  if (displayInVisibleViewport) {
    effectiveViewportRect = {
      width: window.innerWidth,
      height: window.innerHeight,
    }
  }
}

// After: accepts viewport as parameter
function pureCalculateAnchoredPosition(..., settings: PositionSettings, visibleViewportSize: Size) {
  if (displayInVisibleViewport) {
    effectiveViewportRect = {
      width: visibleViewportSize.width,
      height: visibleViewportSize.height,
    }
  }
}

The function is now truly pure and DOM-free as documented.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Feb 2, 2026

⚠️ No Changeset found

Latest commit: 765619d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

…as parameter

Co-authored-by: francinelucca <40550942+francinelucca@users.noreply.github.com>
Copilot AI changed the title [WIP] WIP address feedback on getAnchoredPosition for floatingElements Refactor pureCalculateAnchoredPosition to eliminate DOM dependency Feb 2, 2026
Copilot AI requested a review from francinelucca February 2, 2026 03:53
@francinelucca francinelucca marked this pull request as ready for review February 2, 2026 03:54
@francinelucca francinelucca requested a review from a team as a code owner February 2, 2026 03:54
@francinelucca francinelucca added the skip changeset Skip the check for changesets label Feb 2, 2026
@francinelucca francinelucca merged commit 6dc3606 into chore/getanchoredposition-error-repo Feb 2, 2026
9 of 10 checks passed
@francinelucca francinelucca deleted the copilot/sub-pr-681 branch February 2, 2026 03:55
francinelucca added a commit that referenced this pull request Feb 5, 2026
…ide dialogs (#681)

* add repro

* lock scroll

* implement new "displayInVisibleViewport" setting for getAnchoredPosition

* rename story

* Fix getAnchoredPosition for better dialog support

Enhance getAnchoredPosition to support floatingElements in dialogs with a new setting.

* Refactor pureCalculateAnchoredPosition to eliminate DOM dependency (#683)

* Initial plan

* Refactor pureCalculateAnchoredPosition to accept viewport dimensions as parameter

Co-authored-by: francinelucca <40550942+francinelucca@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: francinelucca <40550942+francinelucca@users.noreply.github.com>

* rename displayInVisibleViewport to displayInViewport

* Update quick-masks-camp.md

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip changeset Skip the check for changesets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants