Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix popup positioning #1151

Merged
merged 8 commits into from
Nov 28, 2023
Merged

Conversation

christianbaroni
Copy link
Member

Fixes BX-872

What changed (plus any additional context for devs)

  • Fixes the positioning of the popup window that appears when connecting or signing — should now correctly position to the top right of the active browser window
  • Adjusts the popup height determination to take into account the estimated size of the popup window's title bar, based on the userAgent

Screen recordings / screenshots

Screenshot 2023-11-23 at 3 01 01 PM

What to test

  • When connecting or signing, the popup window should be positioned to the top right of the active browser window
  • Popups should now appear on the correct screen if external monitors are being used
  • The popup window should be sized correctly and should not be scrollable, at least on Macs

Copy link

linear bot commented Nov 23, 2023

BX-872 Popup window bounds should be related to current screen

From Christian:

have noticed though when I have my laptop open and plugged into my monitor, the popup always opens on my laptop screen, even if the chrome window I’m using is on my monitor (maybe unrelated to this)

This is because we call chrome.windows.create with hardcoded top & left. and those should be calculated relative to the current window instead

Did a quick search for the height of the popup title bar on each platform — may need to adjust these values
Copy link

Here's the packed extension for this build:
rainbowbx-c7c439c05c690da1d40f61aa1fd56af3bf43450b.zip

Copy link

Here's the packed extension for this build:
rainbowbx-6427146f90f45c609c3dafe15d947d34ba2c3251.zip

Copy link
Contributor

@greg-schrammel greg-schrammel left a comment

Choose a reason for hiding this comment

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

looks good
I think arc saves the popup dimensions how you last arranged it, but chrome is right
didn't test with an external monitor

Copy link

Here's the packed extension for this build:
rainbowbx-222bfb5cb60dd2267af0d2998fa5f7c41464d819.zip

Copy link

Here's the packed extension for this build:
node_modules.tar.gz
rainbowbx-9d69bc37900de5ab8b6d28b7be8d27421e85984c.zip

@DanielSinclair
Copy link
Collaborator

DanielSinclair commented Nov 27, 2023

Working perfectly on my DisplayLink monitors. Nice 👌

Copy link

Here's the packed extension for this build:
rainbowbx-3a9436d6ad00e92da6e9d5b43f2607dbf943603b.zip

Copy link

Here's the packed extension for this build:
rainbowbx-e26d048d837fca927dc5f7a80600573cc8434f5b.zip

@christianbaroni christianbaroni merged commit 55d424b into master Nov 28, 2023
17 checks passed
@christianbaroni christianbaroni deleted the @christian/fix-popup-positioning branch November 28, 2023 09:11
Copy link

Here's the packed extension for this build:
node_modules.tar.gz
rainbowbx-55d424bd159fb7a8b913f32fcc02c5a10d5b112d.zip

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.

4 participants