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

Set initial size and position of secondary windows #13201

Merged
merged 13 commits into from
Dec 21, 2023

Conversation

vladarama
Copy link
Contributor

@vladarama vladarama commented Dec 20, 2023

What it does

This PR implements the initial size and positions of secondary windows. It adds a preference called Secondary Window Placement allowing the user to choose the placement of the secondary windows between:

  • originalSize (default): same size as the original widget.
  • halfSize: half the size of the running Theia application
  • fullSize: the same size as the running Theia application.

In addition, another preference (alwaysOnTop) is introduced that allows the user to decide if they want the secondary window to always stay above all other windows, including those of different applications.

How to test

  1. Open Theia electron or browser application.
  2. Install the drawio extension.
  3. Modify the Secondary Window Placement preference.
  4. Create a new test.drawio file or create a new terminal and open it as a secondary window.
  5. The secondary window' size and placement should match the chosen preference.
  6. Repeat steps 3-5 with the other preferences.

Follow-ups

The secondary window popup might not line up perfectly with the original widget since the API used to get the window dimensions has inconsistencies across different browsers and operating systems.

Review checklist

Reminder for reviewers

marcdumais-work and others added 12 commits June 9, 2023 16:52
e.g. sized the same as the original widget that we are extracting into it's own
windows and position at the same place, with an offset.

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
This prototype implements the initial size and positions of secondary
windows. It also adds a preference, called `Secondary Window Placement`
allowing the user to choose the placement of the secondary windows between:
- originalSize: same size as the widget.
- splitScreen: half the size of the Theia application, positioned to the
  side.
- fullScreen: the secondary window will take up the full screen.

Signed-off-by: Vlad Arama <vlad.arama@ericsson.com>
This commit implements the correct values for the initial size and
position of secondary windows.

Signed-off-by <vlad.arama@ericsson.com>
Signed-off-by: Vlad Arama <vlad.arama@ericsson.com>
Signed-off-by: Vlad Arama <vlad.arama@ericsson.com>
Copy link
Contributor

@jfaltermeier jfaltermeier left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I've tested on Ubuntu with the electron and the browser application and it works very well overall. I've added a few comments where the implementation did not match what I expected as a user.

@vince-fugnitto vince-fugnitto added the electron issues related to the electron target label Dec 21, 2023
This commit does the following changes:
- rename `halfSize` to `halfWidth`
- set `alwaysOnTop` to false by default

Signed-off-by: Vlad Arama <vlad.arama@ericsson.com>
Copy link
Contributor Author

@vladarama vladarama left a comment

Choose a reason for hiding this comment

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

@jfaltermeier
Thanks for the review,
I have addressed your comments and the implementation should now match the user's expectations 🚀

Copy link
Contributor

@jfaltermeier jfaltermeier left a comment

Choose a reason for hiding this comment

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

Thanks, lgtm!

@jfaltermeier jfaltermeier merged commit 94103a2 into eclipse-theia:master Dec 21, 2023
14 checks passed
@vince-fugnitto vince-fugnitto added this to the 1.45.0 milestone Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electron issues related to the electron target
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants