react-native icon indicating copy to clipboard operation
react-native copied to clipboard

feat(alert): use support library for alert dialog

Open DomiR opened this issue 3 years ago • 15 comments

Summary

Instead of using the AlertDialog from the android library we use the AlertDialog from the support library to render dialogs properly on Lollipop phones.

Changelog

[Android] [Fixed] - Fix AlertDialog for older android devices by using AlertDialog from the androidx support library

Test Plan

Visual inspection. With old AlertDialog: ddd1692b-2bcb-424a-9355-1b54515c6618 - Donald Duck (1)

With compat AlertDialog: Screenshot_1973-06-21-21-23-28

DomiR avatar Sep 02 '20 14:09 DomiR

Hi @DomiR!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

facebook-github-bot avatar Sep 02 '20 14:09 facebook-github-bot

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

facebook-github-bot avatar Sep 02 '20 15:09 facebook-github-bot

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 02fed063408bb9902dc6df79903b3cf7c82df84e

analysis-bot avatar Sep 02 '20 15:09 analysis-bot

cc @dulmandakh

safaiyeh avatar Sep 02 '20 15:09 safaiyeh

I don't know how to fix the BUCK files... circleci doesn't seem to build properly, but I compiled it on my machine and the result can be seen above.

DomiR avatar Sep 02 '20 22:09 DomiR

I can't wait for the next release and I'm scared to roll with my own fork for my app release 😅

Here https://github.com/DomiR/react-native-compat-alert is reimplementation of the module, that uses androidx.appcompat.app.AlertDialog. Would be nice to see this in the next release.

DomiR avatar Sep 02 '20 23:09 DomiR

Please remove everything in provided_deps, and add react_native_dep("third-party/android/androidx:appcompat")

dulmandakh avatar Sep 03 '20 04:09 dulmandakh

@dulmandakh there is still some BUCK file for the test build that needs updating... any ideas?

DomiR avatar Sep 03 '20 21:09 DomiR

You find a file that fails to compile, and usually there is a BUCK file in the same folder which might require change.

dulmandakh avatar Sep 04 '20 00:09 dulmandakh

I dug a little bit, and couldn't a fix the test failure. It seems like AppCompat components require themed context, ThemedReactContext in RN case, but RN passes ReactApplicationContext to modules constructor which is used by DialogModule to show AlertDialog. Thus tests are failing. Please help @mdvacca

dulmandakh avatar Sep 04 '20 21:09 dulmandakh

Any updates? I have an app that is used a lot on devices with android 5/lollipop, this improvement would help me a lot. Thanks!

camilossantos2809 avatar Jan 06 '21 13:01 camilossantos2809

Sucks that internal Facebook build tools are not the OS way. It builds for me, but BUCK needs to be configured differently to allow those tests to pass.

In the meantime you can use my fork, which only replaces that import, with the one from androidx: https://github.com/DomiR/react-native-compat-alert

DomiR avatar Jan 06 '21 14:01 DomiR

What versions of Android does this aim to fix?

safaiyeh avatar Jan 06 '21 16:01 safaiyeh

I think Lollipop and earlier... other components also use the androidx compact lib, so it's not something unusual. Without compat the Alert works on older phones with the limitations mentioned above (no vertical button stacking, overflow on small devices).

DomiR avatar Jan 06 '21 18:01 DomiR

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Jul 07 '23 05:07 github-actions[bot]

This PR was closed because it has been stalled for 7 days with no activity.

github-actions[bot] avatar Jul 14 '23 05:07 github-actions[bot]