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.

Sign up for GitHub

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

Jump to bottom

Push react native to 0.73.6 #7863

Merged
merged 16 commits into from
Apr 22, 2024
Merged

Push react native to 0.73.6 #7863

merged 16 commits into from
Apr 22, 2024

Conversation

larkox
Copy link
Contributor

@larkox larkox commented Mar 13, 2024 •
edited
Loading

Summary

Push react native dependency and other dependencies that were needed for this push.

Ticket Link

https://mattermost.atlassian.net/browse/MM-57195
May fix https://mattermost.atlassian.net/browse/MM-57114

Release Note

IMPORTANT: updated the IPhone Deployment Target to 13.4. Old devices may not be able to update the app (<1GB RAM and <iPhone6s)

Not sure if we want to rephrase this. In this link on Supported Devices there is a list of the devices that lose support in this version.

@larkox larkox requested a review from enahum March 13, 2024 16:13
@larkox larkox added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Mar 13, 2024
enahum
enahum requested changes Mar 14, 2024
View reviewed changes
Copy link
Contributor

@enahum enahum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We should document that jdk 17 is now needed in the dev docs
  2. iOS is not compiling on Xcode 15.3 due to an issue with @react-native-community/datetimepicker see When running in Xcode 15.3 getting Incompatible function pointer types passing 'YGSize (...)' exception react-native-datetimepicker/datetimepicker#866

android/app/src/debug/java/com/mattermost/flipper/ReactNativeFlipper.java Outdated Show resolved Hide resolved
android/gradlew Outdated Show resolved Hide resolved
patches/@mattermost+react-native-network-client+1.5.0.patch Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes for the network-client should be done directly in the library and the library bumped to a new version, then upgrade the version in package.json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sorry about that. This was an interim change to have the PR working and reviewable. Will update the network library.

patches/@mattermost+react-native-network-client+1.5.0.patch Outdated Show resolved Hide resolved
patches/react-native-navigation+7.38.3.patch Outdated Show resolved Hide resolved
patches/react-native-video+5.2.1.patch Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the previous patches found here https://github.com/mattermost/mattermost-mobile/blob/main/patches/react-native-video%2B5.2.1.patch are no longer needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow what you mean here. In theory I just added a few patches on top of what existed. Every existing patch should still be in the file.

ios/Mattermost.xcodeproj/project.pbxproj Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
app/hooks/gallery.ts
@@ -14,18 +14,6 @@ import {useGallery} from '@context/gallery';
import type {Context, GestureHandlers, OnGestureEvent} from '@typings/screens/gallery';
import type {GestureHandlerGestureEvent} from 'react-native-gesture-handler';

function useRemoteContext<T extends object>(initialValue: T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gallery screen is not dismissing correctly when swiping either up or down on Android

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird, I cannot reproduce. It seems to dismiss just fine for me.

@enahum
Copy link
Contributor

enahum commented Apr 1, 2024

@larkox conflicts

@larkox larkox mentioned this pull request Apr 8, 2024
Merged
@larkox larkox requested a review from enahum April 12, 2024 11:18
larkox
larkox commented Apr 12, 2024
View reviewed changes
patches/react-native-navigation+7.39.1.patch
Comment on lines -76 to -83
@@ -134,6 +130,7 @@ public class NavigationActivity extends AppCompatActivity implements DefaultHard

@Override
public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
+ super.onRequestPermissionsResult(requestCode, permissions, grantResults);
NavigationApplication.instance.onRequestPermissionsResult(requestCode, permissions, grantResults);
if (mPermissionListener != null && mPermissionListener.onRequestPermissionsResult(requestCode, permissions, grantResults)) {
mPermissionListener = null;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change has been removed because it is already upstream.

enahum
enahum approved these changes Apr 16, 2024
View reviewed changes
Copy link
Contributor

@enahum enahum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks really good, great job

question: there are new library updates, should we update them with this PR or do you prefer to push to the next dep updates

@enahum enahum removed the 2: Dev Review Requires review by a core commiter label Apr 16, 2024
@larkox
Copy link
Contributor Author

larkox commented Apr 16, 2024

@enahum Let's update them in the next round of updates. This PR has already taken too long 😛

@larkox
Copy link
Contributor Author

larkox commented Apr 16, 2024

@yasserfaraazkhan This updates many libraries, so a full smoke tests and maybe running the e2e tests we have would be awesome.

No functionality should have changed, but since it is a dependency update "anything" could break.

@yasserfaraazkhan yasserfaraazkhan added the Build Apps for PR Build the mobile app for iOS and Android to test label Apr 16, 2024
yasserfaraazkhan
yasserfaraazkhan approved these changes Apr 21, 2024
View reviewed changes
Copy link
Contributor

@yasserfaraazkhan yasserfaraazkhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zephyr link

Executed mobile test cycle.

@larkox larkox added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Apr 22, 2024
@larkox larkox merged commit 9d1030a into mattermost:main Apr 22, 2024
8 of 10 checks passed
@amyblais amyblais added this to the v2.17.0 milestone Apr 22, 2024
@cpoile cpoile mentioned this pull request Apr 30, 2024
Merged
4 tasks
@marianunez marianunez mentioned this pull request May 6, 2024
Merged
@larkox larkox mentioned this pull request May 7, 2024
Merged
@amyblais amyblais added the Docs/Needed Requires documentation label May 8, 2024
@amyblais amyblais mentioned this pull request May 17, 2024
Open
cyrusjc pushed a commit to cyrusjc/mattermost-mobile that referenced this pull request May 18, 2024
@larkox @cyrusjc
Push react native to 0.73.6 (mattermost#7863)
d0c8307 
* Bump react native to 0.73.6

* iOS changes

* Fix gallery

* Fix test

* Add missing patch

* Unify react native navigation patch

* Update the rest of libraries

* iOS updates

* Update mattermost libraries

* Fix tests and final bumps

* iOS podfile update

* Update android locks

* Revert webrtc update because it was messing with the tests

* Update podfile for webrtc
@cwarnermm cwarnermm added Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels Jun 14, 2024
tdwio pushed a commit to Infomaniak/mobile-kchat that referenced this pull request Aug 9, 2024
@larkox
Push react native to 0.73.6 (mattermost#7863)
1405bc2 
* Bump react native to 0.73.6

* iOS changes

* Fix gallery

* Fix test

* Add missing patch

* Unify react native navigation patch

* Update the rest of libraries

* iOS updates

* Update mattermost libraries

* Fix tests and final bumps

* iOS podfile update

* Update android locks

* Revert webrtc update because it was messing with the tests

* Update podfile for webrtc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enahum enahum enahum approved these changes

@yasserfaraazkhan yasserfaraazkhan yasserfaraazkhan approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Build Apps for PR Build the mobile app for iOS and Android to test Docs/Done Required documentation has been written release-note
Projects
None yet
Milestone
v2.17.0
Development

Successfully merging this pull request may close these issues.
6 participants
@larkox @enahum @yasserfaraazkhan @amyblais @mm-cloud-bot @cwarnermm