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

Adds support for new iOS 13 traits #1568

Merged
merged 2 commits into from
Jan 2, 2020

Conversation

ay8s
Copy link
Collaborator

@ay8s ay8s commented Jul 2, 2019

Adds userInterfaceLevel, accessibilityContrast and legibilityWeight available in iOS 13.

While looking into this I spotted that NSStringFromASPrimitiveTraitCollection (ASTraitCollection.mm) is inserting a nil value for preferredContentSizeCategory under iOS 13 when rotating a device. Similar to what we were seeing here... #754

We could create a AS_NSString method to handle it or check if it doesn't exist and just pass add UIContentSizeCategoryUnspecified.

@ay8s ay8s requested review from maicki and Adlai-Holler July 2, 2019 22:11
@ghost
Copy link

ghost commented Jul 2, 2019

🚫 CI failed with log

@nguyenhuy
Copy link
Member

Note to reviewers: Blocked by Xcode 11 GM and CI upgrade.

@noamalffasy
Copy link

@nguyenhuy Xcode 11 GM is out now

@rahul-malik
Copy link
Contributor

@ay8s - Hey just checking in on this since it'll be helpful for folks building for iOS 13. Can you rebase / resolve conflicts?

@ay8s
Copy link
Collaborator Author

ay8s commented Sep 18, 2019

@rahul-malik Away on a retreat this week so can take a look next week. Feel free to pick up if you need it sooner.

@nguyenhuy nguyenhuy added this to To do in 2.9 Oct 25, 2019
@nguyenhuy nguyenhuy removed this from To do in 2.9 Oct 25, 2019
@rahul-malik
Copy link
Contributor

@ay8s - Ping :)

# Conflicts:
#	Source/Details/ASTraitCollection.h
#	Source/Details/ASTraitCollection.mm
@rahul-malik rahul-malik merged commit 005e2d5 into TextureGroup:master Jan 2, 2020
@nguyenhuy
Copy link
Member

nguyenhuy commented Jan 2, 2020

FYI, I think this PR causes the podspec linting CI job to fail:

- ERROR | [Texture/Core, Texture/PINRemoteImage, Texture/IGListKit, and more...] xcodebuild: Returned an unsuccessful exit code. You can use `--verbose` for more information.

- ERROR | [Texture/Core, Texture/PINRemoteImage, Texture/IGListKit, and more...] xcodebuild:  /Users/runner/runners/2.163.1/work/Texture/Texture/Source/Details/ASTraitCollection.h:51:3: error: 'UIUserInterfaceLevel' is unavailable: not available on tvOS

- ERROR | [Texture/Core, Texture/PINRemoteImage, Texture/IGListKit, and more...] xcodebuild:  /Users/runner/runners/2.163.1/work/Texture/Texture/Source/Details/ASTraitCollection.h:155:22: error: 'UIUserInterfaceLevel' is unavailable: not available on tvOS

- ERROR | [tvOS] [Texture/TextNode2] xcodebuild:  /Users/runner/runners/2.163.1/work/Texture/Texture/Source/Details/ASTraitCollection.h:155:22: error: 
- NOTE  | [tvOS] [Texture/TextNode2] xcodebuild:  note: 'setInsetsLayoutMarginsFromSafeArea:' has been marked as being introduced in tvOS 11.0 here, but the deployment target is tvOS 9.0.0

https://github.com/TextureGroup/Texture/runs/371568518

https://github.com/TextureGroup/Texture/commit/3c8acbc1de9c54b60a020464d5bc777e9a4a3181/checks/371568518/logs

P/S: Perhaps we should run podspec lint on every pull request to catch issues earlier? Right now it's run on master only because the task is really slow.

@jparise
Copy link
Member

jparise commented Jan 3, 2020

Perhaps we should run podspec lint on every pull request to catch issues earlier? Right now it's run on master only because the task is really slow.

Good idea. I think it would be worth it.

nguyenhuy added a commit to nguyenhuy/Texture that referenced this pull request Jan 3, 2020
- UIUserInterfaceLevel was added to ASTraitCollection in TextureGroup#1568. However, it's unavailable on tvOS and thus broke our podspec linting CI job. Fix by only include it if the build target is iOS.
rahul-malik pushed a commit that referenced this pull request Jan 3, 2020
- UIUserInterfaceLevel was added to ASTraitCollection in #1568. However, it's unavailable on tvOS and thus broke our podspec linting CI job. Fix by only include it if the build target is iOS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants