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

An assortment of changes and additions to help with accessibility, alerts, tables, etc #2

Open
wants to merge 36 commits into
base: develop
Choose a base branch
from

Conversation

pkclsoft
Copy link

This pull request isn't my last set of changes for this, but I thought it was about time I shared what I've done so far. I've been away from my effort to build an iOS/tvOS/macOS way forward in swift that works, but this is where I got to. Accessibility mostly works, and I was able to get basic UITableView functionality to work consistently between macOS/iOS (with iOS being the model).

Im happy to continue finessing this; it's certainly not in the state I had planned to have it in before I shared, but I realise that time is getting away from me.

Added UXRectCorner (UIKit only because macOS doesn't support it)
Added static image() function to UXImage to allow an image with arbitrary content provided by a draw block to be generated.
* Some of the changes to NSTableViewCell and UXTableView-* are placeholders to allow compilation to occur.  They don't actually work yet.  The target is to get the AppKit version of the table view to be a reasonably accurate and code-compatible representation of the UIKit table view. (i.e. a single column of table cells as views that provide their own layouts)
* On AppKit there is no "Swipe" gesture recogniser however there is a mechanism for sensing a swipe.  I've added an easy to access method for this.
* A number of UXBezierPath enhancements to make it useful across both platforms.
* Early attempt to build a UIAlert style code-compatibility layer for AppKit.  Incomplete and untested.
…ing of a table row.

Fixed UXIndexPath so that "row" works as expected.
…so that text fields can use that, providing delegate transparency to the application.
…I expect will be common to most UXTableViews regardless of the platform.
* 'develop' of https://github.com/pkclsoft/UXKit:
  Add `UXWindow`
  A few more TextField compat helpers
  Add `UXTextViewDelegate` alias
  Do NOT use `UIFont.monospacedSystemFont`
  Add UIColor.textColor
  UITextView is a class, not a struct
  Import `UITextView`
  Add a CONTRIBUTING.md
  Add `string` property to `UITextView`
  NSTextStorageEditActions requires macOS 10.11+
  Expose NSTextStorageEditActions/NSTextStorage.EditActions
  Add userFixedPitchFont(ofSize:) to UIKit
  Reexport NSUnderlineStyle
  Xcode 13.1 upgrade markers ...
  Make it buildable on tvOS.
  Xcode 13 upgrade markers

# Conflicts:
#	Sources/UXKit/Common/UXTextView.swift
#	Sources/UXKit/UIKit/UXGestures-UIKit.swift
#	Sources/UXKit/UIKit/UXView-UIKit.swift
@helje5
Copy link
Member

helje5 commented Nov 15, 2021

This is a big one, thanks a lot! :-) I'm a little busy right now, but going to go over it soon.

@pkclsoft
Copy link
Author

No problems. I know it's a lot of changes, and as I mentioned, there are perhaps some things in there not quite as polished as I wanted them to be before sharing, but it was beginning to feel like I'd never get there. I'm still chasing the dream, trying to get parity between UIKit and AppKit Accessibility.

@martindufort
Copy link
Contributor

This look like a lot of work that would be beneficial for this library.
Before I send my own PR perhaps this one should be merged so we are not doing the work twice.

@helje5 Would appreciate if you can review and merge... Thanks

# Conflicts:
#	Sources/UXKit/AppKit/UXGraphics-AppKit.swift
#	UXKit.xcodeproj/project.pbxproj
@pkclsoft
Copy link
Author

Thanks for the prompt. I had a bunch of other changes sitting in a side branch that I'd been working from since this PR didn't get their traction I'd hope for. I've now merged those into my develop so that this PR can benefit from those as well.

@helje5
Copy link
Member

helje5 commented Dec 23, 2023

It fails building the Swift package, I'll have a look.

@helje5 helje5 self-assigned this Dec 23, 2023
Copy link
Member

@helje5 helje5 left a comment

Choose a reason for hiding this comment

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

This has a lot of great stuff. Really hard to process in one mega PR :-)

I guess I'd have to go over stuff an cherry-pick and also cleanup the formatting (I don't care about the specific formatting being used, but it should be consistent).

@@ -5,7 +5,7 @@ import PackageDescription
let package = Package(
name: "UXKit",
platforms: [
.macOS(.v10_12), .iOS(.v10)
.macOS(.v10_13), .iOS(.v13), .tvOS(.v13)
Copy link
Member

Choose a reason for hiding this comment

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

That is quite a bump :-) But I think I'm fine w/ that. I think I've actually bumped Xcode even further.

@@ -21,7 +21,7 @@
*
* NOTE: This one doesn't actually support an image :-/
*/
open class NSTableViewCell : NSTableCellView {
open class NSTableViewCell : NSTableCellView, NSTextFieldDelegate {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what to make of that (and the related changes below, including the 1st responder changes). The behaviour added seems specific to a certain use-case, not required for UIKit/AppKit compatibility?
Can you elaborate why we need that in UXKit?


return v
let v = UXLabel(frame: UXRect())
v.cell = VerticallyCenteredTextFieldCell()
Copy link
Member

Choose a reason for hiding this comment

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

Is the centering of all axis really what UITableView does?
I'd also move the Cell implementation into this function and not make it public API.

}

// Override this if you want to receive swipe gestures on your NSView.
func swipeGestureRecognized(inDirection direction: SwipeDirection) {
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't that func have to be open or @objc or sth to be actually subclassed?

The whole swipe thing doesn't seem to have any peer in UIKit? It is more like a convenience addition to just AppKit?

path.addCurve(to: points[2], control1: points[0], control2: points[1])
case .closePath:
path.closeSubpath()
case .cubicCurveTo:
Copy link
Member

Choose a reason for hiding this comment

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

This is completely misaligned and doesn't compile w/ Xcode 14.2. I think to get this to work, we'd need #if swift(5.x.x) checks for both code pathes.
Also macOS 14 now supports this? We should probably use it if available. I.e. this probably needs to be converted to an uxCGPath, call cgPath on iOS and macOS 14, and then do the implementation as required for older macOS.

@@ -111,4 +153,24 @@
}

}

public class UXCustomRowView: NSTableRowView {
Copy link
Member

Choose a reason for hiding this comment

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

No UIKit peer, seems to be a convenience function not belonging into UXKit.

}
}

func setSubViews(enabled: Bool) {
Copy link
Member

Choose a reason for hiding this comment

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

Those should be private, only used by isUserInteractionEnabled. Or maybe the imps should just go into the setter/getter of that. Either way works for me, but let's only add API for things required for compatibility.

}
set {
if let nv = newValue {
self.stringValue = nv
Copy link
Member

Choose a reason for hiding this comment

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

nv ?? "" maybe :-)

@@ -79,4 +79,14 @@
public var uxChildren : [ UXViewController ] { return children }
public func uxRemoveFromParent() { removeFromParent() }
public var uxParent : UXViewController? { return parent }

#if os(macOS)
public func dismiss(animated flag: Bool, completion: (() -> Void)? = nil) {
Copy link
Member

Choose a reason for hiding this comment

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

Not calling completion seems dangerous here.

}

public func present(_ viewControllerToPresent: UXViewController, animated flag: Bool, completion: (() -> Void)? = nil) {
self.presentAsModalWindow(viewControllerToPresent)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe that should also have modalPresentationStyle then (e.g. this also seems to support popovers in UIKit). Also the safer default variant seems to be presenting as sheets (because they can nest)?

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.

3 participants