Skip to content

[iOS][ActivityIndicator] Remove deprecated uses of UIActivityIndicatorViewStyle - #38208

Closed
Saadnajmi wants to merge 1 commit into
react:mainfrom
Saadnajmi:activity-indicator
Closed

[iOS][ActivityIndicator] Remove deprecated uses of UIActivityIndicatorViewStyle#38208
Saadnajmi wants to merge 1 commit into
react:mainfrom
Saadnajmi:activity-indicator

Conversation

@Saadnajmi

Copy link
Copy Markdown
Contributor

Summary:

Super simple change to replace some deprecated ENUM values with the correct one, now that we're on iOS 13.0+.

From UIActivityIndicator.h:

typedef NS_ENUM(NSInteger, UIActivityIndicatorViewStyle) {
UIActivityIndicatorViewStyleMedium API_AVAILABLE(ios(13.0), tvos(13.0)) = 100,
UIActivityIndicatorViewStyleLarge API_AVAILABLE(ios(13.0), tvos(13.0)) = 101,
UIActivityIndicatorViewStyleWhiteLarge API_DEPRECATED_WITH_REPLACEMENT("UIActivityIndicatorViewStyleLarge", ios(2.0, 13.0), tvos(9.0, 13.0)) = 0,
UIActivityIndicatorViewStyleWhite API_DEPRECATED_WITH_REPLACEMENT("UIActivityIndicatorViewStyleMedium", ios(2.0, 13.0), tvos(9.0, 13.0)) = 1,
UIActivityIndicatorViewStyleGray API_DEPRECATED_WITH_REPLACEMENT("UIActivityIndicatorViewStyleMedium", ios(2.0, 13.0)) API_UNAVAILABLE(tvos) = 2,
};

Changelog:

[IOS] [CHANGED] - Remove deprecated uses of UIActivityIndicatorViewStyle

Test Plan:

CI should pass

@facebook-github-botfacebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Microsoft Partner: Microsoft Partner labels Jul 6, 2023
@Saadnajmi
Saadnajmiforce-pushed the activity-indicator branch from 65d4eea to 7025022CompareJuly 6, 2023 04:04
@analysis-bot

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a9,061,545+1
androidhermesarmeabi-v7a8,310,884+2
androidhermesx869,577,765+2
androidhermesx86_649,420,187+1
androidjscarm64-v8a9,613,324-1
androidjscarmeabi-v7a8,739,961+1
androidjscx869,700,289+2
androidjscx86_649,946,841-1

Base commit: 5f84d73
Branch: main

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@rshest has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@github-actions

Copy link
Copy Markdown

This pull request was successfully merged by @Saadnajmi in 62e9fae.

When will my fix make it into a release? | Upcoming Releases

@github-actionsgithub-actionsBot added the Merged This PR has been merged. label Jul 6, 2023
@Saadnajmi
Saadnajmi deleted the activity-indicator branch September 27, 2023 13:49
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 29, 2024
Summary:
Super simple change to replace some deprecated ENUM values with the correct one, now that we're on iOS 13.0+.
From `UIActivityIndicator.h`:
```
typedef NS_ENUM(NSInteger, UIActivityIndicatorViewStyle) {
UIActivityIndicatorViewStyleMedium API_AVAILABLE(ios(13.0), tvos(13.0)) = 100,
UIActivityIndicatorViewStyleLarge API_AVAILABLE(ios(13.0), tvos(13.0)) = 101,
UIActivityIndicatorViewStyleWhiteLarge API_DEPRECATED_WITH_REPLACEMENT("UIActivityIndicatorViewStyleLarge", ios(2.0, 13.0), tvos(9.0, 13.0)) = 0,
UIActivityIndicatorViewStyleWhite API_DEPRECATED_WITH_REPLACEMENT("UIActivityIndicatorViewStyleMedium", ios(2.0, 13.0), tvos(9.0, 13.0)) = 1,
UIActivityIndicatorViewStyleGray API_DEPRECATED_WITH_REPLACEMENT("UIActivityIndicatorViewStyleMedium", ios(2.0, 13.0)) API_UNAVAILABLE(tvos) = 2,
};
```
## Changelog:
[IOS] [CHANGED] - Remove deprecated uses of UIActivityIndicatorViewStyle
Pull Request resolved: react#38208
Test Plan: CI should pass
Reviewed By: cipolleschi
Differential Revision: D47254035
Pulled By: rshest
fbshipit-source-id: 6d3270899e8d52611d91c777f324c3c6f0c520be
Saadnajmi added a commit to microsoft/react-native-macos that referenced this pull request Jan 29, 2024
* Remove deprecated uses of UIActivityIndicatorViewStyle (react#38208)
Summary:
Super simple change to replace some deprecated ENUM values with the correct one, now that we're on iOS 13.0+.
From `UIActivityIndicator.h`:
```
typedef NS_ENUM(NSInteger, UIActivityIndicatorViewStyle) {
UIActivityIndicatorViewStyleMedium API_AVAILABLE(ios(13.0), tvos(13.0)) = 100,
UIActivityIndicatorViewStyleLarge API_AVAILABLE(ios(13.0), tvos(13.0)) = 101,
UIActivityIndicatorViewStyleWhiteLarge API_DEPRECATED_WITH_REPLACEMENT("UIActivityIndicatorViewStyleLarge", ios(2.0, 13.0), tvos(9.0, 13.0)) = 0,
UIActivityIndicatorViewStyleWhite API_DEPRECATED_WITH_REPLACEMENT("UIActivityIndicatorViewStyleMedium", ios(2.0, 13.0), tvos(9.0, 13.0)) = 1,
UIActivityIndicatorViewStyleGray API_DEPRECATED_WITH_REPLACEMENT("UIActivityIndicatorViewStyleMedium", ios(2.0, 13.0)) API_UNAVAILABLE(tvos) = 2,
};
```
## Changelog:
[IOS] [CHANGED] - Remove deprecated uses of UIActivityIndicatorViewStyle
Pull Request resolved: react#38208
Test Plan: CI should pass
Reviewed By: cipolleschi
Differential Revision: D47254035
Pulled By: rshest
fbshipit-source-id: 6d3270899e8d52611d91c777f324c3c6f0c520be
* Refactor RCTActivityIndicator
* Remove TARGET_OS_UIKITFORMAC macros (react#42278)
Summary:
There seems to be a lot of `TARGET_OS_UIKITFORMAC` macro in React Native that don't need to be there. Let's remove them.
First off, what is `TARGET_OS_UIKITFORMAC` targeting? You might think it's [Mac Catalyst](https://developer.apple.com/mac-catalyst/), if you look at the [commit](react@3724810) introducing the ifdefs. However.. that doesn't seem right because `TARGET_OS_MACCATALYST` exists, and is used elsewhere in the codebase. In fact, if you look at this handy comment inside `TargetConditionals.h` (the file that defines all these conditionals), `TARGET_OS_UIKITFORMAC` is not even on there!
```
/*
* TARGET_OS_*
*
* These conditionals specify in which Operating System the generated code will
* run. Indention is used to show which conditionals are evolutionary subclasses.
*
* The MAC/WIN32/UNIX conditionals are mutually exclusive.
* The IOS/TV/WATCH/VISION conditionals are mutually exclusive.
*
* TARGET_OS_WIN32 - Generated code will run on WIN32 API
* TARGET_OS_WINDOWS - Generated code will run on Windows
* TARGET_OS_UNIX - Generated code will run on some Unix (not macOS)
* TARGET_OS_LINUX - Generated code will run on Linux
* TARGET_OS_MAC - Generated code will run on a variant of macOS
* TARGET_OS_OSX - Generated code will run on macOS
* TARGET_OS_IPHONE - Generated code will run on a variant of iOS (firmware, devices, simulator)
* TARGET_OS_IOS - Generated code will run on iOS
* TARGET_OS_MACCATALYST - Generated code will run on macOS
* TARGET_OS_TV - Generated code will run on tvOS
* TARGET_OS_WATCH - Generated code will run on watchOS
* TARGET_OS_VISION - Generated code will run on visionOS
* TARGET_OS_BRIDGE - Generated code will run on bridge devices
* TARGET_OS_SIMULATOR - Generated code will run on an iOS, tvOS, watchOS, or visionOS simulator
* TARGET_OS_DRIVERKIT - Generated code will run on macOS, iOS, tvOS, watchOS, or visionOS
*
* TARGET_OS_EMBEDDED - DEPRECATED: Use TARGET_OS_IPHONE and/or TARGET_OS_SIMULATOR instead
* TARGET_IPHONE_SIMULATOR - DEPRECATED: Same as TARGET_OS_SIMULATOR
* TARGET_OS_NANO - DEPRECATED: Same as TARGET_OS_WATCH
*
* +--------------------------------------------------------------------------------------+
* | TARGET_OS_MAC |
* | +-----+ +------------------------------------------------------------+ +-----------+ |
* | | | | TARGET_OS_IPHONE | | | |
* | | | | +-----------------+ +----+ +-------+ +--------+ +--------+ | | | |
* | | | | | IOS | | | | | | | | | | | | |
* | | OSX | | | +-------------+ | | TV | | WATCH | | BRIDGE | | VISION | | | DRIVERKIT | |
* | | | | | | MACCATALYST | | | | | | | | | | | | | |
* | | | | | +-------------+ | | | | | | | | | | | | |
* | | | | +-----------------+ +----+ +-------+ +--------+ +--------+ | | | |
* | +-----+ +------------------------------------------------------------+ +-----------+ |
* +--------------------------------------------------------------------------------------+
*/
```
Going even deeper into `TargetConditionals.h`, you will see `TARGET_OS_UIKITFORMAC` defined... and it's always 1 when `TARGET_OS_MACCATALYST` is 1, making it feel even more redundant. My current conclusion is it's either another variant of Mac Catalyst (the one where they just run unmodified UIKit maybe..), or it's an older macro back from when Catalyst was still experimental.
Either way, it's pretty obvious nobody is running or testing this codepath, and it adds bloat, especially to React Native macOS where we have extra ifdef blocks for macOS support (and eventually visionOS support). Let's remove it.
Another change I made while we're here:
I've seen this lingering TODO to replace setTargetRect:InView: / setMenuVisible:animated: (deprecated as of iOS 13, below our minimum OS requirement) with showMenuFromView (deprecated as of iOS 16, in line with the availability check). Let's just.... do that?
[IOS] [REMOVED] - Remove TARGET_OS_UIKITFORMAC macros
Pull Request resolved: react#42278
Test Plan:
RNTester with Mac Catalyst still compiles:
![Screenshot 2024-01-15 at 12 26 03 AM](https://github.com/facebook/react-native/assets/6722175/015bd37d-f536-43c7-9586-96187cdbd013)
Reviewed By: cipolleschi
Differential Revision: D52780690
Pulled By: sammy-SC
fbshipit-source-id: df6a333e8e15f79de0ce6f538ebd73b92698dcb6
* Remove an early return to suppress a deprecated API warning for `UIMenuController` (react#42277)
Summary:
`UIMenuController` is deprecated as of iOS 16. react@e08a197 migrated a usage into an `available` check. However, it does not properly fall back to the deprecated API in the "else" block of the availability check, instead it uses an early return. It seems this means Xcode still sees the API as used, and spits out a deprecated warning. Let's just refactor the code so we don't have that anymore.
[IOS] [FIXED] - Remove an early return to suppress a deprecated API warning for `UIMenuController`
Pull Request resolved: react#42277
Test Plan: CI should pass.
Reviewed By: cipolleschi
Differential Revision: D52785488
Pulled By: sammy-SC
fbshipit-source-id: 0b47e8aa8d7c94728e3d68332fbb8f97f8ded34e
* DeviceInfo: Simplify RCTExportedDimensions's API
Summary:
RCTExportedDimensions doesn't need access to the ModuleRegistry, or the bridge. It just uses those two things to get the fontScale.
We could make RCTExportedDimensions easier to understand, by making it do fewer things (i.e: computing the fontScale up front, and passing it into RCTExportedDimensions). Let's just do that.
Changelog: [Internal]
Reviewed By: cipolleschi
Differential Revision: D48237715
fbshipit-source-id: b3af648d88276846742d0e1192d33d180ee49dbb
* iOS: trigger didUpdateDimensions event when resizing without changing traits (react#37649)
Summary:
- when using the `useWindowDimensions()` hook in a component, it's possible for the hook to report incorrect sizes and not update as frequently as it should
- this is most applicable to apps built for iPad and macOS
- closesreact#36118
- either when resizing a React Native app to a different [Size Class](https://developer.apple.com/design/human-interface-guidelines/layout#Device-size-classes) or changing the Appearance, we dispatch an `RCTUserInterfaceStyleDidChangeNotification` notification
- these are then handled in the `interfaceFrameDidChange` method of `RCTDeviceInfo`
- this results in a `didUpdateDimensions` Device Event, which in turn updates the results of `useWindowDimensions()`
- see [RCTDeviceInfo.mm#L217-L232](https://github.com/facebook/react-native/blob/v0.72.0-rc.4/packages/react-native/React/CoreModules/RCTDeviceInfo.mm#L217-L232)
- and [Dimensions.js#L119-L124](https://github.com/facebook/react-native/blob/v0.72.0-rc.4/packages/react-native/Libraries/Utilities/Dimensions.js#L119-L124)
🐛 **However** 🐛
- if you are resizing without triggering a `UITraitCollection` change, the Dimensions reported by `useWindowDimensions()` can become stale, until you either:-
- hit a certain width that is considered a different Size Class
- change the Appearance
- background then resume the app
- make the app full-screen
- added a new `RCTRootViewFrameDidChangeNotification` notification
- the thinking here is to avoid additional overhead by re-using the same `RCTUserInterfaceStyleDidChangeNotification`
- maybe it's overkill?
- the new notifications are sent from an override of `setFrame` on `RCTRootView`
- the new notifications call the same `interfaceFrameDidChange` method of `RCTDeviceInfo`
- Dimensions are now reported correctly when resizing; even within the same Size Class
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[IOS] [FIXED] - Dimensions could be reported incorrectly when resizing iPad or macOS apps
Pull Request resolved: react#37649
Test Plan:
**Reproduction: https://github.com/jpdriver/Dimensions**
or to recreate it yourself:-
- Generate a new project
- Change App.tsx
```
import * as React from 'react';
import {View, Text, useWindowDimensions} from 'react-native';
export default function App() {
const {width, height} = useWindowDimensions();
return (
<View style={{flex: 1, justifyContent: 'center', alignItems: 'center'}}>
<Text>Width: {width}</Text>
<Text>Height: {height}</Text>
</View>
);
}
```
- Open the iOS project in Xcode and enable iPad support
- Enable the iPad app to be used in any orientation
- Run the app
- Enable Stage Manager
- Drag one of the corners to resize the app
Reviewed By: javache
Differential Revision: D46335537
Pulled By: NickGerleman
fbshipit-source-id: 1533f511cf3805fdc9629a2ee115cc00e204d82c
* Refactor RCTDeviceInfo
* Rename `RCTRootViewFrameDidChangeNotification` as it's not trac… (react#39835)
Summary:
…king root view frame changes
Looking through where this was introduced (react#37649), it seems the notification went from tracking root view size changes to window size changes. However, it was not renamed. I was using it for root view changes in RN-macOS, which.. I guess I'll refactor. Meanwhile, let's update the name?
## Changelog:
[IOS] [CHANGED] - Rename `RCTRootViewFrameDidChangeNotification` as it's not tracking root view frame changes
Pull Request resolved: react#39835
Test Plan: CI should pass
Reviewed By: cipolleschi
Differential Revision: D50173742
Pulled By: javache
fbshipit-source-id: 4651696174c439800984a5e6cf642200bb9c4f3c
* Default dimensions `scale` value to 1.0 if window is nil (#1970)
* [0.73] Default scale to 1.0 on if window is nil
* Unify RCTExportedDimensions between iOS & macOS
* Use typedef instead of #define for NSWindow
* Fix issues from cherry-picks
* Add visionOS support
* Extra changes for visionOS on 0.72-stable
* Use Xcode 15.2
---------
Co-authored-by: Ramanpreet Nara <ramanpreet@meta.com>
Co-authored-by: JP <jpdriver@users.noreply.github.com>
Co-authored-by: Shawn Dempsey <96719+shwanton@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.p: MicrosoftPartner: MicrosoftPartner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Saadnajmi@analysis-bot@facebook-github-bot@rshest