[AppKit] Add some of the missing NSAccessibility bits by iainx · Pull Request #79 · dotnet/macios

iainx · 2016-05-25T17:17:28Z

No description provided.

dnfclas · 2016-05-25T17:17:32Z

Hi @iainx, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

spouliot · 2016-05-25T17:49:06Z

+		[DllImport (Constants.AppKitLibrary)]
+		static extern IntPtr NSAccessibilityRoleDescription (IntPtr role, IntPtr subrole);
+
+		public static string RoleDescription (NSString role, NSString subrole)


a method name needs a verb, e.g. GetRoleDescription

spouliot · 2016-05-25T18:02:14Z

@iainx thanks for the PR :) my 2 comments (method name and null check) needs to be applied globally.

Also can you confirm that you ran the introspection tests with those changes ? that will spot some potential defects (including incorrect Apple documentation). If you did just reply here, thanks!

Also @chamons might have more feedback too...

xamarin-release-manager · 2016-05-25T18:35:22Z

Build success

chamons · 2016-05-25T20:53:23Z

+		[DllImport (Constants.AppKitLibrary)]
+		static extern IntPtr NSAccessibilityActionDescription (IntPtr action);
+
+		public static string ActionDescription (NSString action)


Another "needs a verb" in name

chamons · 2016-05-25T20:54:54Z

I picked out the places where @spouliot's name change needs hit. Once you land those then LGTM.

spouliot · 2016-05-25T21:11:54Z

+	[Category, BaseType (typeof (NSObject))]
+	public interface NSObjectAccessibilityExtensions {
+		[Export ("accessibilityHitTest:")]
+		NSObject AccessibilityHitTest (CGPoint point);


same for binding categories, methods needs verbs :)

iainx · 2016-05-26T16:36:54Z

I've fixed all the bits mentioned and I've run the introspection tests.

I changed the NSObjectAccessibilityExtensions and NSWorkspaceAccessibilityExtensions to be a protocol instead of a category because it's really an informal protocol implemented as a category.

I also added all the obsolete methods because in standard Apple style they've obsoleted things without giving a better way of doing that task, so it looks like the obsolete methods will still be needed for certain things.

xamarin-release-manager · 2016-05-26T17:15:12Z

Build success

rolfbjarne · 2016-05-27T15:26:11Z

My comments have been solved to my satisfaction, and I have nothing else 👍

spouliot · 2016-05-27T15:41:57Z

+
+	[Protocol, BaseType (typeof (NSObject))]
+	public interface NSObjectAccessibilityExtensions {
+		[Obsolete]


That's not really helpful for developers as it does not say why ? and what's the recommended replacement ?

In most cases that's better handled with something like:

[Availability (Obsoleted = Platform.Mac_10_10, Message = "Use XXX instead")]

as it tells when it was obsoleted and how to update the code (when possible). IIRC you mentioned some things were obsolete without an alternative and, if that's the case, then it should become the text, e.g. "No alternative is documented".

Also in some cases it's Deprecated and not Obsoleted that should be used (or both).

iainx · 2016-05-31T13:43:52Z

I've fixed the Obsolete attributes, directing people at the newer protocols, which is all Apple's docs do too, as there's no direct equivalent (it seems they're wanting to do things in a different way from 10.10)

I'm not sure what the difference between Obsoleted and Deprecated is so maybe I should have used Deprecated.

xamarin-release-manager · 2016-05-31T13:51:18Z

Build success

chamons · 2016-05-31T17:20:25Z

I agree with @rolfbjarne that everything looks reasonable now. Landing.

spouliot · 2016-06-06T13:18:30Z

@chamons dontlink-mac fails since this was merged https://wrench.internalx.com/Wrench/ViewLane.aspx?lane_id=1381&host_id=243&revision_id=768118

iainx · 2016-06-06T23:14:28Z

@spouliot @chamons yes, sorry the accessibilityAttributeNames shouldn't have the :
I'll make a new PR to fix that tomorrow

iainx · 2016-06-06T23:14:49Z

Is it possible to run these tests locally?

chamons · 2016-06-07T21:25:01Z

@iainx see #143

and make run-mac-dontlink in tests dir

New commits in spouliot/Touch.Unit: * xamarin/Touch.Unit@9266732 [Touch.Client] Add support for macOS. (dotnet#80) * xamarin/Touch.Unit@2d556a5 [TouchRunner] Fix network logging to log if we're logging to a file even if EnableNetwork is false. (dotnet#79) * xamarin/Touch.Unit@1f85e16 [TouchOptions] Fix a few issues related to command-line parsing. (dotnet#78) * xamarin/Touch.Unit@1c272a9 [TouchRunner] Rename TestElement.Update to TestElement.TestFinished to make it clearer when it's used. (dotnet#73) * xamarin/Touch.Unit@9a654ca [TouchRunner] Fix running multiple test assemblies in NUnitLite mode. (dotnet#76) * xamarin/Touch.Unit@55c747f [TouchRunner] Print test output to the console. (dotnet#75) * xamarin/Touch.Unit@ab5dce0 [TouchRunner] Keep track of the last shown test suite, and return to it when the app is relaunched. (dotnet#74) * xamarin/Touch.Unit@1a7068c [TouchRunner] Only update the UI for a single test after the test has finished running. (dotnet#72) * xamarin/Touch.Unit@ba93ddf [TouchRunner] Fix unused variable warning by only defining the variable when it's used. (dotnet#71) * xamarin/Touch.Unit@ab2aaa5 [TouchRunner] Always show 'Run all'. (dotnet#70) * xamarin/Touch.Unit@af58317 Move every Touch.Client projects to its own folder. (dotnet#69) * xamarin/Touch.Unit@8a0ec0d [TouchRunner] Improve name printing in NuGet mode. (dotnet#68) * xamarin/Touch.Unit@c7f8ff0 Ignore any 'packages' directory. (dotnet#67) Diff: https://github.com/spouliot/Touch.Unit/compare/b4e8606a85255f2456b0b20d5a0732298fa1ad36..926673287338482f3949659288ab36001413d251

New commits in spouliot/Touch.Unit: * xamarin/Touch.Unit@dafdf9f [Touch.Client] Add API to exclude tests based on categories. * xamarin/Touch.Unit@e6fb7d7 [Touch.Client] Fix paths in the .NET project files. * xamarin/Touch.Unit@9266732 [Touch.Client] Add support for macOS. (dotnet#80) * xamarin/Touch.Unit@2d556a5 [TouchRunner] Fix network logging to log if we're logging to a file even if EnableNetwork is false. (dotnet#79) * xamarin/Touch.Unit@1f85e16 [TouchOptions] Fix a few issues related to command-line parsing. (dotnet#78) * xamarin/Touch.Unit@1c272a9 [TouchRunner] Rename TestElement.Update to TestElement.TestFinished to make it clearer when it's used. (dotnet#73) * xamarin/Touch.Unit@9a654ca [TouchRunner] Fix running multiple test assemblies in NUnitLite mode. (dotnet#76) * xamarin/Touch.Unit@55c747f [TouchRunner] Print test output to the console. (dotnet#75) * xamarin/Touch.Unit@ab5dce0 [TouchRunner] Keep track of the last shown test suite, and return to it when the app is relaunched. (dotnet#74) * xamarin/Touch.Unit@1a7068c [TouchRunner] Only update the UI for a single test after the test has finished running. (dotnet#72) * xamarin/Touch.Unit@ba93ddf [TouchRunner] Fix unused variable warning by only defining the variable when it's used. (dotnet#71) * xamarin/Touch.Unit@ab2aaa5 [TouchRunner] Always show 'Run all'. (dotnet#70) * xamarin/Touch.Unit@af58317 Move every Touch.Client projects to its own folder. (dotnet#69) * xamarin/Touch.Unit@8a0ec0d [TouchRunner] Improve name printing in NuGet mode. (dotnet#68) * xamarin/Touch.Unit@c7f8ff0 Ignore any 'packages' directory. (dotnet#67) Diff: https://github.com/spouliot/Touch.Unit/compare/b4e8606a85255f2456b0b20d5a0732298fa1ad36..dafdf9f44f8fada4e47e2abd0c95c9e494377a8c

New commits in spouliot/Touch.Unit: * xamarin/Touch.Unit@9266732 [Touch.Client] Add support for macOS. (dotnet#80) * xamarin/Touch.Unit@2d556a5 [TouchRunner] Fix network logging to log if we're logging to a file even if EnableNetwork is false. (dotnet#79) * xamarin/Touch.Unit@1f85e16 [TouchOptions] Fix a few issues related to command-line parsing. (dotnet#78) * xamarin/Touch.Unit@1c272a9 [TouchRunner] Rename TestElement.Update to TestElement.TestFinished to make it clearer when it's used. (dotnet#73) * xamarin/Touch.Unit@9a654ca [TouchRunner] Fix running multiple test assemblies in NUnitLite mode. (dotnet#76) * xamarin/Touch.Unit@55c747f [TouchRunner] Print test output to the console. (dotnet#75) * xamarin/Touch.Unit@ab5dce0 [TouchRunner] Keep track of the last shown test suite, and return to it when the app is relaunched. (dotnet#74) * xamarin/Touch.Unit@1a7068c [TouchRunner] Only update the UI for a single test after the test has finished running. (dotnet#72) * xamarin/Touch.Unit@ba93ddf [TouchRunner] Fix unused variable warning by only defining the variable when it's used. (dotnet#71) * xamarin/Touch.Unit@ab2aaa5 [TouchRunner] Always show 'Run all'. (dotnet#70) * xamarin/Touch.Unit@af58317 Move every Touch.Client projects to its own folder. (dotnet#69) * xamarin/Touch.Unit@8a0ec0d [TouchRunner] Improve name printing in NuGet mode. (dotnet#68) * xamarin/Touch.Unit@c7f8ff0 Ignore any 'packages' directory. (dotnet#67) Diff: https://github.com/spouliot/Touch.Unit/compare/b4e8606a85255f2456b0b20d5a0732298fa1ad36..926673287338482f3949659288ab36001413d251

New commits in spouliot/Touch.Unit: * xamarin/Touch.Unit@dafdf9f [Touch.Client] Add API to exclude tests based on categories. * xamarin/Touch.Unit@e6fb7d7 [Touch.Client] Fix paths in the .NET project files. * xamarin/Touch.Unit@9266732 [Touch.Client] Add support for macOS. (dotnet#80) * xamarin/Touch.Unit@2d556a5 [TouchRunner] Fix network logging to log if we're logging to a file even if EnableNetwork is false. (dotnet#79) * xamarin/Touch.Unit@1f85e16 [TouchOptions] Fix a few issues related to command-line parsing. (dotnet#78) * xamarin/Touch.Unit@1c272a9 [TouchRunner] Rename TestElement.Update to TestElement.TestFinished to make it clearer when it's used. (dotnet#73) * xamarin/Touch.Unit@9a654ca [TouchRunner] Fix running multiple test assemblies in NUnitLite mode. (dotnet#76) * xamarin/Touch.Unit@55c747f [TouchRunner] Print test output to the console. (dotnet#75) * xamarin/Touch.Unit@ab5dce0 [TouchRunner] Keep track of the last shown test suite, and return to it when the app is relaunched. (dotnet#74) * xamarin/Touch.Unit@1a7068c [TouchRunner] Only update the UI for a single test after the test has finished running. (dotnet#72) * xamarin/Touch.Unit@ba93ddf [TouchRunner] Fix unused variable warning by only defining the variable when it's used. (dotnet#71) * xamarin/Touch.Unit@ab2aaa5 [TouchRunner] Always show 'Run all'. (dotnet#70) * xamarin/Touch.Unit@af58317 Move every Touch.Client projects to its own folder. (dotnet#69) * xamarin/Touch.Unit@8a0ec0d [TouchRunner] Improve name printing in NuGet mode. (dotnet#68) * xamarin/Touch.Unit@c7f8ff0 Ignore any 'packages' directory. (dotnet#67) Diff: https://github.com/spouliot/Touch.Unit/compare/b4e8606a85255f2456b0b20d5a0732298fa1ad36..dafdf9f44f8fada4e47e2abd0c95c9e494377a8c

This also required adding System.Core.dll to the MonoTouch.NUnitLite.dll, and fixing a csproj path. New commits in spouliot/Touch.Unit: * xamarin/Touch.Unit@dafdf9f [Touch.Client] Add API to exclude tests based on categories. * xamarin/Touch.Unit@e6fb7d7 [Touch.Client] Fix paths in the .NET project files. * xamarin/Touch.Unit@9266732 [Touch.Client] Add support for macOS. (#80) * xamarin/Touch.Unit@2d556a5 [TouchRunner] Fix network logging to log if we're logging to a file even if EnableNetwork is false. (#79) * xamarin/Touch.Unit@1f85e16 [TouchOptions] Fix a few issues related to command-line parsing. (#78) * xamarin/Touch.Unit@1c272a9 [TouchRunner] Rename TestElement.Update to TestElement.TestFinished to make it clearer when it's used. (#73) * xamarin/Touch.Unit@9a654ca [TouchRunner] Fix running multiple test assemblies in NUnitLite mode. (#76) * xamarin/Touch.Unit@55c747f [TouchRunner] Print test output to the console. (#75) * xamarin/Touch.Unit@ab5dce0 [TouchRunner] Keep track of the last shown test suite, and return to it when the app is relaunched. (#74) * xamarin/Touch.Unit@1a7068c [TouchRunner] Only update the UI for a single test after the test has finished running. (#72) * xamarin/Touch.Unit@ba93ddf [TouchRunner] Fix unused variable warning by only defining the variable when it's used. (#71) * xamarin/Touch.Unit@ab2aaa5 [TouchRunner] Always show 'Run all'. (#70) * xamarin/Touch.Unit@af58317 Move every Touch.Client projects to its own folder. (#69) * xamarin/Touch.Unit@8a0ec0d [TouchRunner] Improve name printing in NuGet mode. (#68) * xamarin/Touch.Unit@c7f8ff0 Ignore any 'packages' directory. (#67) Diff: https://github.com/spouliot/Touch.Unit/compare/b4e8606a85255f2456b0b20d5a0732298fa1ad36..dafdf9f44f8fada4e47e2abd0c95c9e494377a8c

This also required adding System.Core.dll to the MonoTouch.NUnitLite.dll, and fixing a csproj path. New commits in spouliot/Touch.Unit: * xamarin/Touch.Unit@dafdf9f [Touch.Client] Add API to exclude tests based on categories. * xamarin/Touch.Unit@e6fb7d7 [Touch.Client] Fix paths in the .NET project files. * xamarin/Touch.Unit@9266732 [Touch.Client] Add support for macOS. (dotnet#80) * xamarin/Touch.Unit@2d556a5 [TouchRunner] Fix network logging to log if we're logging to a file even if EnableNetwork is false. (dotnet#79) * xamarin/Touch.Unit@1f85e16 [TouchOptions] Fix a few issues related to command-line parsing. (dotnet#78) * xamarin/Touch.Unit@1c272a9 [TouchRunner] Rename TestElement.Update to TestElement.TestFinished to make it clearer when it's used. (dotnet#73) * xamarin/Touch.Unit@9a654ca [TouchRunner] Fix running multiple test assemblies in NUnitLite mode. (dotnet#76) * xamarin/Touch.Unit@55c747f [TouchRunner] Print test output to the console. (dotnet#75) * xamarin/Touch.Unit@ab5dce0 [TouchRunner] Keep track of the last shown test suite, and return to it when the app is relaunched. (dotnet#74) * xamarin/Touch.Unit@1a7068c [TouchRunner] Only update the UI for a single test after the test has finished running. (dotnet#72) * xamarin/Touch.Unit@ba93ddf [TouchRunner] Fix unused variable warning by only defining the variable when it's used. (dotnet#71) * xamarin/Touch.Unit@ab2aaa5 [TouchRunner] Always show 'Run all'. (dotnet#70) * xamarin/Touch.Unit@af58317 Move every Touch.Client projects to its own folder. (dotnet#69) * xamarin/Touch.Unit@8a0ec0d [TouchRunner] Improve name printing in NuGet mode. (dotnet#68) * xamarin/Touch.Unit@c7f8ff0 Ignore any 'packages' directory. (dotnet#67) Diff: https://github.com/spouliot/Touch.Unit/compare/b4e8606a85255f2456b0b20d5a0732298fa1ad36..dafdf9f44f8fada4e47e2abd0c95c9e494377a8c

This also required adding System.Core.dll to the MonoTouch.NUnitLite.dll, and fixing a csproj path. New commits in spouliot/Touch.Unit: * xamarin/Touch.Unit@f620668 [Touch.Client] Adjust project configurations. (dotnet#83) * xamarin/Touch.Unit@545400b [Touch.Client] Fix paths in the .NET project files. (dotnet#81) * xamarin/Touch.Unit@703586b [Touch.Client] Add API to exclude tests based on categories. (dotnet#82) * xamarin/Touch.Unit@9266732 [Touch.Client] Add support for macOS. (dotnet#80) * xamarin/Touch.Unit@2d556a5 [TouchRunner] Fix network logging to log if we're logging to a file even if EnableNetwork is false. (dotnet#79) * xamarin/Touch.Unit@1f85e16 [TouchOptions] Fix a few issues related to command-line parsing. (dotnet#78) * xamarin/Touch.Unit@1c272a9 [TouchRunner] Rename TestElement.Update to TestElement.TestFinished to make it clearer when it's used. (dotnet#73) * xamarin/Touch.Unit@9a654ca [TouchRunner] Fix running multiple test assemblies in NUnitLite mode. (dotnet#76) * xamarin/Touch.Unit@55c747f [TouchRunner] Print test output to the console. (dotnet#75) * xamarin/Touch.Unit@ab5dce0 [TouchRunner] Keep track of the last shown test suite, and return to it when the app is relaunched. (dotnet#74) * xamarin/Touch.Unit@1a7068c [TouchRunner] Only update the UI for a single test after the test has finished running. (dotnet#72) * xamarin/Touch.Unit@ba93ddf [TouchRunner] Fix unused variable warning by only defining the variable when it's used. (dotnet#71) * xamarin/Touch.Unit@ab2aaa5 [TouchRunner] Always show 'Run all'. (dotnet#70) * xamarin/Touch.Unit@af58317 Move every Touch.Client projects to its own folder. (dotnet#69) * xamarin/Touch.Unit@8a0ec0d [TouchRunner] Improve name printing in NuGet mode. (dotnet#68) * xamarin/Touch.Unit@c7f8ff0 Ignore any 'packages' directory. (dotnet#67) Diff: https://github.com/spouliot/Touch.Unit/compare/b4e8606a85255f2456b0b20d5a0732298fa1ad36..f62066880ae548bac9302d714b8c045955051014

dnfclas added the cla-required label May 25, 2016

spouliot reviewed May 25, 2016
View reviewed changes

spouliot added cla-signed and removed cla-required labels May 25, 2016

chamons reviewed May 25, 2016
View reviewed changes

spouliot reviewed May 25, 2016
View reviewed changes

iainx force-pushed the add-some-missing-nsa-bits branch from 28acf16 to 6021640 Compare May 26, 2016 16:34

spouliot reviewed May 27, 2016
View reviewed changes

spouliot assigned chamons May 27, 2016

[AppKit] Add some of the missing NSAccessibility bits

9bb2fca

iainx force-pushed the add-some-missing-nsa-bits branch from 6021640 to 9bb2fca Compare May 31, 2016 13:19

chamons merged commit f535df6 into dotnet:master May 31, 2016

rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Apr 5, 2019

azdevops: dotnet#79.

9bc35c1

Conversation

iainx commented May 25, 2016

Uh oh!

dnfclas commented May 25, 2016

Uh oh!

spouliot May 25, 2016

Choose a reason for hiding this comment

Uh oh!

spouliot commented May 25, 2016

Uh oh!

xamarin-release-manager commented May 25, 2016

Uh oh!

chamons May 25, 2016

Choose a reason for hiding this comment

Uh oh!

chamons commented May 25, 2016

Uh oh!

spouliot May 25, 2016

Choose a reason for hiding this comment

Uh oh!

iainx commented May 26, 2016

Uh oh!

xamarin-release-manager commented May 26, 2016

Uh oh!

rolfbjarne commented May 27, 2016

Uh oh!

spouliot May 27, 2016

Choose a reason for hiding this comment

Uh oh!

iainx commented May 31, 2016

Uh oh!

xamarin-release-manager commented May 31, 2016

Uh oh!

chamons commented May 31, 2016

Uh oh!

spouliot commented Jun 6, 2016

Uh oh!

iainx commented Jun 6, 2016

Uh oh!

iainx commented Jun 6, 2016

Uh oh!

chamons commented Jun 7, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants