[ASTextNode2] Integrate Google's changes to ASTN2 and accessibility #2017
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This time I included the accessibility changes, which required adding some of the new accessibility features in other parts of the code. Note, there are cases where not all of the accessibility was brought over because that was supposed to be covered in a separate PR made by someone from Google.
|
@wiseoldduck Here is a second attempt that includes the accessibility changes. Maybe the first time around i left those out because I thought someone from Google was looking in making an accessibility PR? |
This test passes on master but fails on the merge branch. Looking at the snapshot difference, it appears that google figured out a way to get more characters in before truncating. The text is still within its provided max size.
… collision (TextureGroup#2020) As of iOS15 the AuthenticationServices framework has a class named `ASNavigationController`. We need to rename our `ASNavigationController` to protect against undefined behavior. Note: This change was based on this PR TextureGroup#2014. We were slow in merging it and the author has not replied, so I'm making a new one to get this landed.
With the breaking change of renaming ASNavigationController to ASDKNavigationController, we have released a new version of Texture. Please see `ThreeMigrationGuide.md` for how to handle the breaking changes in 3.1.0.
- The framework isn't available on tvOS. This causes CocoaPods linting to fail which prevented me from pushing the new release out. - One way to fix this is to have a different `default_subspecs` for tvOS that doesn't have AssetsLibrary subspec, but per-platform `default_subspecs` doesn't seem to be supported by CocoaPods. So I updated the subspec itself to only depend on the framework for iOS. This means the subspec is empty/useless for tvOS (and other platforms FWIW). - Tested with `pod spec lint Texture.podspec`. - Fixes TextureGroup#1992 and part of TextureGroup#1549. Also unblocks 3.1.0 release. - For the long term, we can remove the subspec entirely when iOS 9 is deprecated (TextureGroup#1828).
rcancro
force-pushed
the
astextnode2merge
branch
from
1d6dc1b
to
da7a706
Compare
It looks like github updated so that `macos-latest` is now macos-11. It can't find Xcode_11_5. Let's try to set the runs-on version explicitly and see if some magic happens. also allow warnings for podlint because that started failing too
rcancro
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR takes the changes related to
ASTextNode2from #1986 and merges them into master.The largest changes were:
My biggest concern is that there were many changes on master around accessibility that conflicted with what Google implemented. In Google's PR #1986, the changes on master were placed behind these experimental feature checks:
However, since the changes behind these flags are currently live on master, I think this is the behavior that should still be the default on master. I would say that we either add code to opt-in to these experiments in this PR or we switch the logic so that the changes from Google's PR are the "experimental" path.