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

Improvements on WiFiMenuItem #183

Merged
merged 2 commits into from
Nov 22, 2020

Conversation

ErrorErrorError
Copy link
Collaborator

@ErrorErrorError ErrorErrorError commented Aug 27, 2020

Hey guys,

Just here with a few bug fixes in regards to WiFiMenuItemView! I also added a few feature requests, which are #179, #166, and #196.

I also changed an error in the Spanish translation! After GM release of Big Sur is out, I will see if there is a fix for blur issue on items. However now highlight is available.

Ready for review!

@zxystd
Copy link
Collaborator

zxystd commented Aug 28, 2020

good.

@williambj1
Copy link
Collaborator

@ErrorErrorError

I'm extremely sorry for my late response, too busy these days :(

Setting WiFiMenuItemView to NSVisualEffectView instead of NSView leaves a messed up bullring effect in Big Sur but I guess that's the only way to make the highlighting work. I've tried a couple of changes myself and I didn't come up with a perfect solution.

Huge thanks for your hard work! Really appreciated!

image

HeliPort/Appearance/StatusMenu.swift Outdated Show resolved Hide resolved
HeliPort/Appearance/StatusMenu.swift Outdated Show resolved Hide resolved
williambj1 added a commit that referenced this pull request Aug 31, 2020
@ErrorErrorError
Copy link
Collaborator Author

ErrorErrorError commented Aug 31, 2020

I'm extremely sorry for my late response, too busy these days :(

No worries! I've also been busy with school/work so I understand.

Setting WiFiMenuItemView to NSVisualEffectView instead of NSView leaves a messed up bullring effect in Big Sur but I guess that's the only way to make the highlighting work. I've tried a couple of changes myself and I didn't come up with a perfect solution.

Did you by any chance tried using material = .popup rather than material = .menu? I can just also use a custom NSView and draw the highlight manually.

Edit: I'll just wait until Big Sur is released at this point. I'm sure it's not really a big deal as of now.

williambj1 added a commit that referenced this pull request Sep 1, 2020
* Merge localization changes from #183

* i18n: Update Chinese localization

* Update credits
@ErrorErrorError ErrorErrorError marked this pull request as draft September 1, 2020 14:45
@ErrorErrorError ErrorErrorError marked this pull request as ready for review September 28, 2020 20:28
@williambj1
Copy link
Collaborator

williambj1 commented Nov 19, 2020

Hi! @ErrorErrorError, it's been quite a long time and finally we've got the stable release of Big Sur from Apple.

I made some changes based on your PR and the final result looks quite well in Big Sur, would you mind me pushing directly to your branch?

Here are the changes:
diff --git a/HeliPort/Appearance/WiFiMenuItemView.swift b/HeliPort/Appearance/WiFiMenuItemView.swift
index 66a6b31..c9fb54c 100644
--- a/HeliPort/Appearance/WiFiMenuItemView.swift
+++ b/HeliPort/Appearance/WiFiMenuItemView.swift
@@ -16,119 +16,193 @@
 import Foundation
 import Cocoa
 
-class WifiMenuItemView: NSVisualEffectView {
+class WifiMenuItemView: NSView {
 
-    let statusImage: NSImageView = {
+    // - MARK: Public
+
+    public var networkInfo: NetworkInfo {
+        willSet(networkInfo) {
+            ssidLabel.stringValue = networkInfo.ssid
+            lockImage.isHidden = networkInfo.auth.security == ITL80211_SECURITY_NONE
+            signalImage.image = StatusBarIcon.getRssiImage(Int16(networkInfo.rssi))
+            layoutSubtreeIfNeeded()
+        }
+    }
+
+    public var visible: Bool = true {
+        willSet(visible) {
+            isHidden = !visible
+            heightConstraint.constant = visible ? menuBarHeight : 0
+            layoutSubtreeIfNeeded()
+        }
+    }
+
+    public var connected: Bool = false {
+        willSet(connected) {
+            statusImage.isHidden = !connected
+        }
+    }
+
+    public init(networkInfo: NetworkInfo) {
+        self.networkInfo = networkInfo
+        super.init(frame: NSRect(x: 0, y: 0, width: 0, height: menuBarHeight))
+
+        self.addSubview(effectView)
+        effectView.addSubview(statusImage)
+        effectView.addSubview(ssidLabel)
+        effectView.addSubview(lockImage)
+        effectView.addSubview(signalImage)
+
+        setupLayout()
+    }
+
+    required init?(coder: NSCoder) {
+        fatalError("init(coder:) has not been implemented")
+    }
+
+    public func checkHighlight() {
+        if visible, let position = currentWindow?.mouseLocationOutsideOfEventStream {
+            isMouseOver = bounds.contains(convert(position, from: nil))
+        }
+    }
+
+    // - MARK: Private
+
+    private var currentWindow: NSWindow?
+
+    private var heightConstraint: NSLayoutConstraint!
+
+    private let menuBarHeight: CGFloat = {
+        if #available(macOS 11, *) {
+            return 22
+        } else {
+            return 20
+        }
+    }()
+
+    private let effectView: NSVisualEffectView = {
+        let effectView = NSVisualEffectView()
+        effectView.material = .popover
+        effectView.state = .active
+        effectView.isEmphasized = true
+        effectView.blendingMode = .behindWindow
+        return effectView
+    }()
+
+    private let statusImage: NSImageView = {
         let statusImage = NSImageView()
-        statusImage.image = NSImage(named: NSImage.menuOnStateTemplateName)
-        statusImage.isHidden = true
         statusImage.setContentHuggingPriority(.defaultHigh, for: .horizontal)
-        return statusImage
-    }()
+        statusImage.isHidden = true
 
-    let ssidLabel: NSTextField = {
-        let ssidLabel = NSTextField(labelWithString: "")
-        ssidLabel.font = NSFont.systemFont(ofSize: 14)
-        return ssidLabel
+        if #available(OSX 11.0, *) {
+            statusImage.image = NSImage(named: NSImage.menuOnStateTemplateName)?
+                                .withSymbolConfiguration(NSImage.SymbolConfiguration(pointSize: 13,
+                                                                                     weight: .bold,
+                                                                                     scale: .small))
+        } else {
+            statusImage.image = NSImage(named: NSImage.menuOnStateTemplateName)
+        }
+
+        return statusImage
     }()
 
-    let lockImage: NSImageView = {
+    private let lockImage: NSImageView = {
         let lockImage = NSImageView()
-        lockImage.image = NSImage(named: NSImage.lockLockedTemplateName)
         lockImage.setContentHuggingPriority(.defaultHigh, for: .horizontal)
+
+        if #available(OSX 11.0, *) {
+            lockImage.image = NSImage(named: NSImage.lockLockedTemplateName)?
+                              .withSymbolConfiguration(NSImage.SymbolConfiguration(pointSize: 14,
+                                                                                   weight: .semibold,
+                                                                                   scale: .medium))
+        } else {
+            lockImage.image = NSImage(named: NSImage.lockLockedTemplateName)
+        }
+
         return lockImage
     }()
 
-    let signalImage: NSImageView = {
+    private let signalImage: NSImageView = {
         let signalImage = NSImageView()
         signalImage.setContentHuggingPriority(.defaultHigh, for: .horizontal)
         return signalImage
     }()
 
-    var isMouseOver: Bool = false {
-        willSet(hover) {
-            material = hover ? .selection : .menu
-            isEmphasized = hover
-            ssidLabel.textColor = hover ? .white : .textColor
-            if #available(OSX 10.14, *) {
-                statusImage.contentTintColor = hover ? .white : .textColor
-                lockImage.contentTintColor = hover ? .white : .textColor
-                signalImage.contentTintColor = hover ? .white : .textColor
-            }
-        }
-    }
+    private let ssidLabel: NSTextField = {
+        let ssidLabel = NSTextField(labelWithString: "")
 
-    var visible: Bool = true {
-        willSet(visible) {
-            isHidden = !visible
-            heightConstraint.constant = visible ? 19 : 0
-            layoutSubtreeIfNeeded()
+        if #available(macOS 11, *) {
+            ssidLabel.font = NSFont.menuFont(ofSize: 0)
+        } else {
+            ssidLabel.font = NSFont.systemFont(ofSize: 14)
         }
-    }
 
-    var connected: Bool = false {
-        willSet(connected) {
-            statusImage.isHidden = !connected
-        }
-    }
+        return ssidLabel
+    }()
 
-    var currentWindow: NSWindow?
+    private var isMouseOver: Bool = false {
+        willSet(hover) {
+            effectView.material = hover ? .selection : .popover
+            effectView.isEmphasized = hover
 
-    var networkInfo: NetworkInfo {
-        willSet(networkInfo) {
-            ssidLabel.stringValue = networkInfo.ssid
-            lockImage.isHidden = networkInfo.auth.security == ITL80211_SECURITY_NONE
-            signalImage.image = StatusBarIcon.getRssiImage(Int16(networkInfo.rssi))
-            layoutSubtreeIfNeeded()
+            ssidLabel.textColor = hover ? .selectedMenuItemTextColor : .controlTextColor
+
+            statusImage.cell?.backgroundStyle = hover ? .emphasized : .normal
+            lockImage.cell?.backgroundStyle = hover ? .emphasized : .normal
+            signalImage.cell?.backgroundStyle = hover ? .emphasized : .normal
         }
     }
 
-    var heightConstraint: NSLayoutConstraint!
-
-    func setupLayout() {
-        subviews.forEach { $0.translatesAutoresizingMaskIntoConstraints = false }
+    private func setupLayout() {
+
+        let effectPadding: CGFloat
+        let statusPadding: CGFloat
+        let statusWidth: CGFloat
+        let lockWidth: CGFloat
+        if #available(macOS 11, *) {
+            effectView.wantsLayer = true
+            effectView.layer?.cornerRadius = 4
+            effectView.layer?.masksToBounds = true
+            effectPadding = 5
+            statusPadding = 10
+            statusWidth = 15
+            lockWidth = 16
+        } else {
+            effectPadding = 0
+            statusPadding = 6
+            statusWidth = 12
+            lockWidth = 10
+        }
 
-        heightConstraint = heightAnchor.constraint(equalToConstant: 19)
+        heightConstraint = heightAnchor.constraint(equalToConstant: menuBarHeight)
         heightConstraint.priority = NSLayoutConstraint.Priority(rawValue: 1000)
         heightConstraint.isActive = true
 
         statusImage.centerYAnchor.constraint(equalTo: self.centerYAnchor).isActive = true
-        statusImage.leadingAnchor.constraint(equalTo: self.leadingAnchor, constant: 6).isActive = true
-        statusImage.widthAnchor.constraint(equalToConstant: 12).isActive = true
+        statusImage.leadingAnchor.constraint(equalTo: self.leadingAnchor, constant: statusPadding).isActive = true
+        statusImage.widthAnchor.constraint(equalToConstant: statusWidth).isActive = true
 
         ssidLabel.centerYAnchor.constraint(equalTo: self.centerYAnchor).isActive = true
         ssidLabel.leadingAnchor.constraint(equalTo: statusImage.trailingAnchor, constant: 3).isActive = true
-        ssidLabel.topAnchor.constraint(equalTo: self.topAnchor).isActive = true
-        ssidLabel.bottomAnchor.constraint(equalTo: self.bottomAnchor).isActive = true
 
         lockImage.centerYAnchor.constraint(equalTo: self.centerYAnchor).isActive = true
         lockImage.leadingAnchor.constraint(equalTo: ssidLabel.trailingAnchor, constant: 10).isActive = true
-        lockImage.widthAnchor.constraint(equalToConstant: 10).isActive = true
+        lockImage.widthAnchor.constraint(equalToConstant: lockWidth).isActive = true
 
         signalImage.centerYAnchor.constraint(equalTo: self.centerYAnchor, constant: 1).isActive = true
         signalImage.leadingAnchor.constraint(equalTo: lockImage.trailingAnchor, constant: 12).isActive = true
         signalImage.trailingAnchor.constraint(equalTo: self.trailingAnchor, constant: -12).isActive = true
         signalImage.widthAnchor.constraint(equalToConstant: 18).isActive = true
-    }
 
-    init(networkInfo: NetworkInfo) {
-        self.networkInfo = networkInfo
-        super.init(frame: NSRect.zero)
-
-        self.addSubview(statusImage)
-        self.addSubview(ssidLabel)
-        self.addSubview(lockImage)
-        self.addSubview(signalImage)
-        self.material = .menu
-
-        setupLayout()
+        effectView.translatesAutoresizingMaskIntoConstraints = false
+        effectView.subviews.forEach { $0.translatesAutoresizingMaskIntoConstraints = false }
+        effectView.leftAnchor.constraint(equalTo: self.leftAnchor, constant: effectPadding).isActive = true
+        effectView.rightAnchor.constraint(equalTo: self.rightAnchor, constant: -effectPadding).isActive = true
+        effectView.topAnchor.constraint(equalTo: topAnchor).isActive = true
+        effectView.bottomAnchor.constraint(equalTo: bottomAnchor).isActive = true
     }
 
-    func checkHighlight() {
-        if visible, let position = currentWindow?.mouseLocationOutsideOfEventStream {
-            isMouseOver = bounds.contains(convert(position, from: nil))
-        }
-    }
+    // - MARK: Overrides
 
     override func mouseUp(with event: NSEvent) {
         isMouseOver = false // NSWindow pop up could escape mouseExit
@@ -150,7 +224,12 @@ class WifiMenuItemView: NSVisualEffectView {
         checkHighlight()
     }
 
-    required init?(coder: NSCoder) {
-        fatalError("init(coder:) has not been implemented")
+    override func layout() {
+        super.layout()
+        if #available(macOS 11, *) {
+            effectView.frame = CGRect(x: 5, y: 0, width: bounds.width - 10, height: bounds.height)
+        } else {
+            effectView.frame = bounds
+        }
     }
 }

@ErrorErrorError
Copy link
Collaborator Author

ErrorErrorError commented Nov 19, 2020

Hi! @ErrorErrorError, it's been quite a long time and finally we've got the stable release of Big Sur from Apple.

I made some changes based on your PR and the final result looks quite well in Big Sur, would you mind me pushing directly to your branch?

Hey! Sorry I have been busy with school. These changes look great! I will test it later today and looks like I also did minor changes in StatusMenu.swift so I will check if those changes still fix the issues mentioned at the beginning of comment.

Edit: Everything seems to be working well! I just made sure that this repo is still up to date with the upstream. Feel free to merge your commits here or if you want me to add it I can do it too.

- Fixed highlight issue on < 10.16 (macOS 11) But the background will look a bit weird. Will not fix until Big Sur GM is released.
- Shows "Disconnect from " if auto join is not enabled
- Only show "About HeliPort" if Options + WiFi icon is pressed, was a request but not necessary.
- Minor fixes with WiFiMenuItemView.
- Improved showing disconnect menu only if is connected, auto join is not enabled, or if options is pressed and there is a network connection.
- Removed preventing items to refresh on mouse over, will need a separate PR.
- Fixed minor issue with Log.
- Removed colon from "Disconnect from " so it can be consistent with localizations.
@williambj1
Copy link
Collaborator

williambj1 commented Nov 21, 2020

@ErrorErrorError Please have a look :)

Edit: I will update the CI on master so it no longer fails

@williambj1 williambj1 added this to the v1.0.2 milestone Nov 22, 2020
@williambj1 williambj1 merged commit aa8c27b into OpenIntelWireless:master Nov 22, 2020
@williambj1 williambj1 linked an issue Nov 23, 2020 that may be closed by this pull request
@williambj1 williambj1 mentioned this pull request Nov 23, 2020
@williambj1 williambj1 linked an issue Nov 23, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request i18n Interlocalization
3 participants