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

Refactored Window Positioning #2250

Merged
merged 10 commits into from
Jan 18, 2019
Merged

Refactored Window Positioning #2250

merged 10 commits into from
Jan 18, 2019

Conversation

grokys
Copy link
Member

@grokys grokys commented Jan 18, 2019

Based on a comment on #1979 from @kekekeks this PR refactors window positioning a little, and hopefully fixes the bugs we've been seeing around that. It does the following:

  • Make PixelSize accept a scaling factor for conversions: conversions that accept DPI are still present for the moment but with a WithDpi suffix
  • Added PixelPoint and PixelRect to represent device pixel points and rects
  • Use PixelPoint and PixelRect for screen co-ordinates. The affected members are:
    • IWindowBaseImpl.Position
    • IWindowBaseImpl.PositionChanged
    • ITopLevelImpl.PointToClient
    • ITopLevelImpl.PointToScreen
    • IMouseDevice.Position
    • Screen.Bounds
    • Screen.WorkingArea
  • Make ShowDialog respect WindowStartupLocation: supersedes WIP: Make ShowDialog respect WindowStartupLocation. #2246
  • Make Show and ShowDialog respect WindowStartupLocation DPI-awareness: supersedes WindowStartupLocation DPI-awareness #1979
  • Modifies DialogsPage to allow testing of startup location.

Note: this is a breaking change

Still TODO:

  • We really need non-client window size in order to position owned windows correctly
  • We probably want to remove Window.Owner and pass the owner to the Show method as we do for dialogs: having owner specified in different places for dialogs vs non-dialogs is not a good API

Fixes #1961
Fixes #2207

For conversions between `PixelSize` and `Size` use a scaling factor by default; rename the DPI overloads to have a `WithDpi` suffix.
In addition to `PixelSize`.
The affected members are:

- `IWindowBaseImpl.Position`
- `IWindowBaseImpl.PositionChanged`
- `ITopLevelImpl.PointToClient`
- `ITopLevelImpl.PointToScreen`
- `IMouseDevice.Position`
- `Screen.Bounds`
- `Screen.WorkingArea`
`SetWindowStartupLocation` needs to be called _after_ the window is shown as is done in `Show`. In addition, the owner window needs to be passed so we can get the correct screen.

Renamed the `parent` parameter to `ShowDialog` to `owner` as its `WindowStartupLocation.CenterOwner` not `CenterParent`.
We probably want to change this and pass the owner to the `Show` method as we do for dialogs: having owner specified in different places for dialogs vs non-dialogs is not a good API.
@grokys grokys requested a review from a team January 18, 2019 12:29
@grokys grokys changed the title Refactored Window Position Refactored Window Positioning Jan 18, 2019
Copy link
Member

@kekekeks kekekeks left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -236,6 +236,7 @@ private void Open(Control control)

_popup = new PopupRoot { Content = this, };
((ISetLogicalParent)_popup).SetParent(control);

Copy link
Member

Choose a reason for hiding this comment

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

Nit: whitespace

@grokys grokys merged commit 9bd182d into master Jan 18, 2019
@grokys grokys deleted the refactor/window-position branch January 18, 2019 16:47
@grokys grokys added this to the 0.8.0 milestone Apr 3, 2019
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.

ShowDialog() ignores WindowStartupLocation WindowStartupLocation is not HiDPI compatible
2 participants