Remove Window.position

Closes #32.
tmandry · Jan 2, 2019 · 90fdbb9 · 90fdbb9
1 parent 4adaceb
commit 90fdbb9
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 63 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,18 +1,18 @@
 0.0.3
 =====
 
-Breaking changes
-- `Window.position` and the new `Window.frame` now use Cocoa coordinates (origin
-  at bottom-left), to match the behavior of Screen and most modern macOS APIs.
-- `Window.position` was made non-writeable (use `Window.frame` instead). It may
-  be removed in the future. See #29 for more.
-- `WindowFrameChangedEvent` was added, replacing `WindowPosChangedEvent` and
-  `WindowSizeChangedEvent`. See #16 for more.
-
 New features
 - A `Window.frame` property was added. You can now atomically change the whole
   frame of a window.
 
+Breaking changes
+- The new `Window.frame` now uses Cocoa coordinates (origin at bottom-left), to
+  match the behavior of Screen and most modern macOS APIs.
+- `Window.position` was removed in favor of `Window.frame`. See #32 and #29 for
+  more.
+- `WindowFrameChangedEvent` was added, replacing `WindowPosChangedEvent` and
+  `WindowSizeChangedEvent`. See #16 for more.
+
 Bug fixes
 - `ScreenLayoutChangedEvent` is now correctly detected.
 - When a property value is written to, and the new value is changed but does

diff --git a/Sources/Window.swift b/Sources/Window.swift
@@ -41,13 +41,12 @@ public final class Window {
     public var screen: Screen? {
         let screenIntersectSizes =
             application.swindlerState.screens.lazy
-            .map { screen in (screen, screen.frame.intersection(self.rect)) }
+            .map { screen in (screen, screen.frame.intersection(self.frame.value)) }
             .filter { _, intersect in !intersect.isNull }
             .map { screen, intersect in (screen, intersect.size.width * intersect.size.height) }
         let bestScreen = screenIntersectSizes.max { lhs, rhs in lhs.1 < rhs.1 }?.0
         return bestScreen
     }
-    fileprivate var rect: CGRect { return CGRect(origin: position.value, size: size.value) }
 
     /// Whether or not the window referred to by this type remains valid. Windows usually become
     /// invalid because they are destroyed (in which case a WindowDestroyedEvent will be emitted).
@@ -56,10 +55,9 @@ public final class Window {
     public var isValid: Bool { return delegate.isValid }
 
     /// The frame of the window.
+    ///
+    /// The origin of the frame is the bottom-left corner of the window in screen coordinates.
     public var frame: WriteableProperty<OfType<CGRect>> { return delegate.frame }
-    /// The position of the bottom-left corner of the window in screen coordinates.
-    /// To set this, use `frame.origin`. This property may be removed in the future.
-    public var position: Property<OfType<CGPoint>> { return delegate.position }
     /// The size of the window in screen coordinates.
     public var size: WriteableProperty<OfType<CGSize>> { return delegate.size }
 
@@ -103,7 +101,6 @@ protocol WindowDelegate: class {
     var appDelegate: ApplicationDelegate? { get }
 
     var frame: WriteableProperty<OfType<CGRect>>! { get }
-    var position: Property<OfType<CGPoint>>! { get }
     var size: SizeProperty! { get }
     var title: Property<OfType<String>>! { get }
     var isMinimized: WriteableProperty<OfType<Bool>>! { get }
@@ -133,7 +130,6 @@ final class OSXWindowDelegate<
     weak var appDelegate: ApplicationDelegate?
 
     var frame: WriteableProperty<OfType<CGRect>>!
-    var position: Property<OfType<CGPoint>>!
     var size: SizeProperty!
     var title: Property<OfType<String>>!
     var isMinimized: WriteableProperty<OfType<Bool>>!
@@ -158,9 +154,6 @@ final class OSXWindowDelegate<
             withEvent: WindowFrameChangedEvent.self,
             receivingObject: Window.self,
             notifier: self)
-        position = Property(
-            PositionPropertyDelegate(frameDelegate, frame),
-            notifier: self)
         size = SizeProperty(
             AXPropertyDelegate(axElement, .size, initPromise),
             notifier: self,
@@ -186,15 +179,14 @@ final class OSXWindowDelegate<
             isFullscreen
         ]
         let allProperties: [PropertyType] = axProperties + [
-            frame,
-            position
+            frame
         ]
 
         // Map notifications on this element to the corresponding property.
         // Note that `size` implicitly updates every time `frame` updates, so it is not listed here.
         watchedAxProperties = [
-            .moved: [frame, position],
-            .resized: [frame, position, isFullscreen],
+            .moved: [frame],
+            .resized: [frame, isFullscreen],
             .titleChanged: [title],
             .windowMiniaturized: [isMinimized],
             .windowDeminiaturized: [isMinimized]
@@ -366,34 +358,6 @@ private final class FramePropertyDelegate<UIElement: UIElementType>: PropertyDel
     }
 }
 
-/// Adapts from .frame to .position.
-private final class PositionPropertyDelegate<UIElement: UIElementType>: PropertyDelegate {
-    typealias T = CGPoint
-
-    let frameDelegate: FramePropertyDelegate<UIElement>
-    let frame: WriteableProperty<OfType<CGRect>>
-
-    init(_ frameDelegate_: FramePropertyDelegate<UIElement>,
-         _ frame_: WriteableProperty<OfType<CGRect>>) {
-        frameDelegate = frameDelegate_
-        frame = frame_
-    }
-
-    func readValue() throws -> T? {
-        return try frameDelegate.readValue()?.origin
-    }
-
-    func writeValue(_ newValue: T) throws {
-        // This cannot be done atomically, since the top-left coordinate has to be computed from
-        // the size, which may be out of date. Therefore we don't support writing only the position.
-        fatalError("writing to position property is unsupported; shouldn't get here")
-    }
-
-    func initialize() -> Promise<T?> {
-        return frameDelegate.initialize().then { return $0?.origin }
-    }
-}
-
 // MARK: SizeProperty
 
 /// Custom Property class for the `size` property.

diff --git a/SwindlerTests/DelegateStubs.swift b/SwindlerTests/DelegateStubs.swift
@@ -33,7 +33,6 @@ class StubWindowDelegate: WindowDelegate {
     var appDelegate: ApplicationDelegate?
 
     var frame: WriteableProperty<OfType<CGRect>>!
-    var position: Property<OfType<CGPoint>>!
     var size: SizeProperty!
     var title: Property<OfType<String>>!
     var isMinimized: WriteableProperty<OfType<Bool>>!
@@ -47,7 +46,6 @@ class StubWindowDelegate: WindowDelegate {
         let notifier = TestPropertyNotifier()
 
         frame = WriteableProperty(frame_, notifier: notifier)
-        position = WriteableProperty(position_, notifier: notifier)
         size = SizeProperty(size_, notifier: notifier, frame: frame)
     }
 

diff --git a/SwindlerTests/WindowSpec.swift b/SwindlerTests/WindowSpec.swift
@@ -59,9 +59,10 @@ class OSXWindowDelegateInitializeSpec: QuickSpec {
                 return initialize().then { windowDelegate -> Void in
                     // In inverted (AX) coordinates, top-left is (5, 5) and bottom-left is (5, 105)
                     // In Cocoa coordinates, the bottom-left would be (5, 1000 - 105) = (5, 895)
-                    expect(windowDelegate.position.value).to(equal(CGPoint(x: 5, y: 895)))
-                    expect(windowDelegate.size.value).to(equal(CGSize(width: 100, height: 100)))
-                    expect(windowDelegate.title.value).to(equal("a window title"))
+                    expect(windowDelegate.frame.value.origin) == CGPoint(x: 5, y: 895)
+                    expect(windowDelegate.frame.value.size) == CGSize(width: 100, height: 100)
+                    expect(windowDelegate.size.value) == CGSize(width: 100, height: 100)
+                    expect(windowDelegate.title.value) == "a window title"
                     expect(windowDelegate.isMinimized.value).to(beFalse())
                     expect(windowDelegate.isFullscreen.value).to(beFalse())
                 }
@@ -378,10 +379,10 @@ class OSXWindowDelegateSpec: QuickSpec {
 
             describe("position") {
                 it("updates when the window is moved") {
-                    expect(windowDelegate.position.value).to(equal(CGPoint(x: 0, y: 900)))
+                    expect(windowDelegate.frame.value.origin).to(equal(CGPoint(x: 0, y: 900)))
                     windowElement.attrs[.position] = CGPoint(x: 1, y: 1)
                     windowDelegate.handleEvent(.moved, observer: TestObserver())
-                    expect(windowDelegate.position.value).toEventually(equal(CGPoint(x: 1, y: 899)))
+                    expect(windowDelegate.frame.value.origin).toEventually(equal(CGPoint(x: 1, y: 899)))
                 }
 
                 it("updates when the window is resized from the top") {
@@ -391,15 +392,15 @@ class OSXWindowDelegateSpec: QuickSpec {
                     windowDelegate.handleEvent(.resized, observer: TestObserver())
                     expect(windowDelegate.size.value).toEventually(equal(
                         CGSize(width: 100, height: 75)))
-                    expect(windowDelegate.position.value).toEventually(equal(
+                    expect(windowDelegate.frame.value.origin).toEventually(equal(
                         CGPoint(x: 0, y: 900)))  // no change
                 }
 
                 it("updates when the window is resized from the bottom") {
                     windowElement.attrs[.position] = CGPoint(x: 0, y: 0)
                     windowElement.attrs[.size]     = CGSize(width: 100, height: 75)
                     windowDelegate.handleEvent(.resized, observer: TestObserver())
-                    expect(windowDelegate.position.value).toEventually(equal(
+                    expect(windowDelegate.frame.value.origin).toEventually(equal(
                         CGPoint(x: 0, y: 925)))
                     expect(windowDelegate.size.value).toEventually(equal(
                         CGSize(width: 100, height: 75)))
@@ -500,12 +501,11 @@ class WindowSpec: QuickSpec {
             }
 
             func setWindowRect(_ rect: CGRect) {
-                windowDelegate.position_.value = rect.origin
                 windowDelegate.frame_.value = rect
                 waitUntil { done in
-                    when(fulfilled: windowDelegate.position.refresh(),
-                                    windowDelegate.frame.refresh())
-                        .then { _ in done() }.always {}
+                    windowDelegate.frame.refresh()
+                        .then { _ in done() }
+                        .always {}
                 }
             }