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

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

Closed
wants to merge 1 commit into from

Conversation

Saadnajmi
Copy link
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-bot facebook-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
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 9,061,545 +1
android hermes armeabi-v7a 8,310,884 +2
android hermes x86 9,577,765 +2
android hermes x86_64 9,420,187 +1
android jsc arm64-v8a 9,613,324 -1
android jsc armeabi-v7a 8,739,961 +1
android jsc x86 9,700,289 +2
android jsc x86_64 9,946,841 -1

Base commit: 5f84d73
Branch: main

@facebook-github-bot
Copy link
Contributor

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

@github-actions
Copy link

github-actions bot commented Jul 6, 2023

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

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

@github-actions github-actions bot 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: facebook#38208

Test Plan: CI should pass

Reviewed By: cipolleschi

Differential Revision: D47254035

Pulled By: rshest

fbshipit-source-id: 6d3270899e8d52611d91c777f324c3c6f0c520be
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: facebook#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 (facebook#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: facebook#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 (facebook#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](facebook@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: facebook#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` (facebook#42277)

Summary:
`UIMenuController` is deprecated as of iOS 16. facebook@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: facebook#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 (facebook#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
- closes facebook#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: facebook#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… (facebook#39835)

Summary:
…king root view frame changes

Looking through where this was introduced (facebook#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: facebook#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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Microsoft Partner: Microsoft Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants